firedtv: add parameter to fake ca_system_ids in CA_INFO
Commit Message
Date: Sun, 4 Oct 2009 15:00:36 +0200
From: Henrik Kurelid <henrik@kurelid.se>
The Digital Everywhere firmware have the shortcoming that ca_info_enq and
ca_info are not supported. This means that we can never retrieve the correct
ca_system_id to present in the CI message CA_INFO. Currently the driver uses
the application id retrieved using app_info_req and app_info, but this id
only match the correct ca_system_id as given in ca_info in some cases.
This patch adds a parameter to the driver in order for the user to override
what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
be specified.
This is needed for users with CAMs that have different manufacturer id and
ca_system_id and that uses applications that take this into account, like
MythTV.
Signed-off-by: Henrik Kurelid <henrik@kurelid.se>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (rebased, format of comment)
---
Would be good to get into an .34-rc too.
drivers/media/dvb/firewire/firedtv-avc.c | 31 ++++++++++++++++++++---
1 file changed, 27 insertions(+), 4 deletions(-)
Comments
> The Digital Everywhere firmware have the shortcoming that ca_info_enq and
> ca_info are not supported. This means that we can never retrieve the correct
> ca_system_id to present in the CI message CA_INFO. Currently the driver uses
> the application id retrieved using app_info_req and app_info, but this id
> only match the correct ca_system_id as given in ca_info in some cases.
> This patch adds a parameter to the driver in order for the user to override
> what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
> be specified.
> This is needed for users with CAMs that have different manufacturer id and
> ca_system_id and that uses applications that take this into account, like
> MythTV.
This workaround is of course rather awkward. Users who need this will
have a hard time to work out that this parameter needs to be set and
how. Furthermore, the web site of Digital Everywhere says that hey
stopped manufacturing and ramped down support, hence it looks like this
issue will stay with us forever.
Isn't there a CA command that could be (mis)used for some kind of
probing of the CAM for correct system IDs? E.g. a loop which retries a
kind of dummy operation with different system IDs until success,
initially during fdtv_dvb_register/ fdtv_ca_register?
(I don't know how CI works, and alas I don't have a CAM myself for
testing and am not very keen on getting one...)
Stefan Richter wrote:
> The Digital Everywhere firmware have the shortcoming that ca_info_enq and
> ca_info are not supported. This means that we can never retrieve the correct
> ca_system_id to present in the CI message CA_INFO. Currently the driver uses
> the application id retrieved using app_info_req and app_info, but this id
> only match the correct ca_system_id as given in ca_info in some cases.
> This patch adds a parameter to the driver in order for the user to override
> what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
> be specified.
> This is needed for users with CAMs that have different manufacturer id and
> ca_system_id and that uses applications that take this into account, like
> MythTV.
This seems an ugly workaround. The better seems to patch MythTV to accept a different
CAM.
> +static int num_fake_ca_system_ids;
...
> + for (i = 0; i < num_fake_ca_system_ids; i++) {
> + app_info[4 + i * 2] =
> + (fake_ca_system_ids[i] >> 8) & 0xff;
...
NAK. If someone put an arbitrary high value for num_fake_ca_system_id's, it will write outside
the app_info array space, as the num_fake_ca_system_ids is not validated against the size
of app_info. Also, it makes no sense a negative value for this parameter.
Mauro Carvalho Chehab wrote:
> Stefan Richter wrote:
>
>> The Digital Everywhere firmware have the shortcoming that ca_info_enq and
>> ca_info are not supported. This means that we can never retrieve the correct
>> ca_system_id to present in the CI message CA_INFO. Currently the driver uses
>> the application id retrieved using app_info_req and app_info, but this id
>> only match the correct ca_system_id as given in ca_info in some cases.
>> This patch adds a parameter to the driver in order for the user to override
>> what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
>> be specified.
>> This is needed for users with CAMs that have different manufacturer id and
>> ca_system_id and that uses applications that take this into account, like
>> MythTV.
>
> This seems an ugly workaround. The better seems to patch MythTV to accept a different
> CAM.
Ugly it is, for sure. Can't comment on application-level solutions; if
thats the proper layer at which to address this, then that would be
preferable of course.
>> +static int num_fake_ca_system_ids;
> ...
>> + for (i = 0; i < num_fake_ca_system_ids; i++) {
>> + app_info[4 + i * 2] =
>> + (fake_ca_system_ids[i] >> 8) & 0xff;
> ...
>
> NAK. If someone put an arbitrary high value for num_fake_ca_system_id's, it will write outside
> the app_info array space, as the num_fake_ca_system_ids is not validated against the size
> of app_info.
That's what I thought at first look at the patch too, but then I noticed
that inlcude/linux/moduleparam.h and kernel/params.c properly track
kparam_arry.max = ARRAY_SIZE(array).
http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L62
http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L213
http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L351
http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L296
So no danger here.
> Also, it makes no sense a negative value for this parameter.
I already posted an updated version of the patch which correctly defines
num_fake_ca_system_ids as an unsigned long.
Stefan Richter wrote:
> I already posted an updated version of the patch which correctly defines
> num_fake_ca_system_ids as an unsigned long.
err, unsigned int of course, as http://patchwork.kernel.org/patch/82912/.
Stefan Richter wrote:
> Mauro Carvalho Chehab wrote:
>> Stefan Richter wrote:
>>
>>> The Digital Everywhere firmware have the shortcoming that ca_info_enq and
>>> ca_info are not supported. This means that we can never retrieve the correct
>>> ca_system_id to present in the CI message CA_INFO. Currently the driver uses
>>> the application id retrieved using app_info_req and app_info, but this id
>>> only match the correct ca_system_id as given in ca_info in some cases.
>>> This patch adds a parameter to the driver in order for the user to override
>>> what will be returned in the CA_INFO CI message. Up to four ca_system_ids can
>>> be specified.
>>> This is needed for users with CAMs that have different manufacturer id and
>>> ca_system_id and that uses applications that take this into account, like
>>> MythTV.
>> This seems an ugly workaround. The better seems to patch MythTV to accept a different
>> CAM.
>
> Ugly it is, for sure. Can't comment on application-level solutions; if
> thats the proper layer at which to address this, then that would be
> preferable of course.
From the report, this seems to be a requirement at application level. So, the fix should
be there.
Henrik, is there any reason why not patching MythTV?
>
>>> +static int num_fake_ca_system_ids;
>> ...
>>> + for (i = 0; i < num_fake_ca_system_ids; i++) {
>>> + app_info[4 + i * 2] =
>>> + (fake_ca_system_ids[i] >> 8) & 0xff;
>> ...
>>
>> NAK. If someone put an arbitrary high value for num_fake_ca_system_id's, it will write outside
>> the app_info array space, as the num_fake_ca_system_ids is not validated against the size
>> of app_info.
>
> That's what I thought at first look at the patch too, but then I noticed
> that inlcude/linux/moduleparam.h and kernel/params.c properly track
> kparam_arry.max = ARRAY_SIZE(array).
> http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L62
> http://lxr.linux.no/#linux+v2.6.33/include/linux/moduleparam.h#L213
> http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L351
> http://lxr.linux.no/#linux+v2.6.33/kernel/params.c#L296
>
> So no danger here.
>
>> Also, it makes no sense a negative value for this parameter.
>
> I already posted an updated version of the patch which correctly defines
> num_fake_ca_system_ids as an unsigned long.
Ok. As app_info is char [256] (from ca_info::msg field), and kernel module avoids
the size of the array to be bigger than 4 (the sizeof the array), that's should be ok.
Stefan Richter wrote:
> Stefan Richter wrote:
>> I already posted an updated version of the patch which correctly defines
>> num_fake_ca_system_ids as an unsigned long.
>
> err, unsigned int of course, as http://patchwork.kernel.org/patch/82912/.
Ah, ok. This is one of the things that Patchwork doesn't handle nice: it doesn't show
any hint that a patch might be superseded by a newer one.
===================================================================
@@ -130,6 +130,20 @@ MODULE_PARM_DESC(debug, "Verbose logging
", FCP payloads = " __stringify(AVC_DEBUG_FCP_PAYLOADS)
", or a combination, or all = -1)");
+/*
+ * This is a workaround since there is no vendor specific command to retrieve
+ * ca_info using AVC. If this parameter is not used, ca_system_id will be
+ * filled with application_manufacturer from ca_app_info.
+ * Digital Everywhere have said that adding ca_info is on their TODO list.
+ */
+static int num_fake_ca_system_ids;
+static int fake_ca_system_ids[4] = { -1, -1, -1, -1 };
+module_param_array(fake_ca_system_ids, int, &num_fake_ca_system_ids, 0644);
+MODULE_PARM_DESC(fake_ca_system_ids, "If your CAM application manufacturer "
+ "does not have the same ca_system_id as your CAS, you can "
+ "override what ca_system_ids are presented to the "
+ "application by setting this field to an array of ids.");
+
static const char *debug_fcp_ctype(unsigned int ctype)
{
static const char *ctypes[] = {
@@ -977,7 +991,7 @@ int avc_ca_info(struct firedtv *fdtv, ch
{
struct avc_command_frame *c = (void *)fdtv->avc_data;
struct avc_response_frame *r = (void *)fdtv->avc_data;
- int pos, ret;
+ int i, pos, ret;
mutex_lock(&fdtv->avc_mutex);
@@ -1004,9 +1018,18 @@ int avc_ca_info(struct firedtv *fdtv, ch
app_info[0] = (EN50221_TAG_CA_INFO >> 16) & 0xff;
app_info[1] = (EN50221_TAG_CA_INFO >> 8) & 0xff;
app_info[2] = (EN50221_TAG_CA_INFO >> 0) & 0xff;
- app_info[3] = 2;
- app_info[4] = r->operand[pos + 0];
- app_info[5] = r->operand[pos + 1];
+ if (num_fake_ca_system_ids == 0) {
+ app_info[3] = 2;
+ app_info[4] = r->operand[pos + 0];
+ app_info[5] = r->operand[pos + 1];
+ } else {
+ app_info[3] = num_fake_ca_system_ids * 2;
+ for (i = 0; i < num_fake_ca_system_ids; i++) {
+ app_info[4 + i * 2] =
+ (fake_ca_system_ids[i] >> 8) & 0xff;
+ app_info[5 + i * 2] = fake_ca_system_ids[i] & 0xff;
+ }
+ }
*len = app_info[3] + 4;
out:
mutex_unlock(&fdtv->avc_mutex);