[PATCH] ir-core: Fix null dereferences in the protocols sysfs interface

2010-09-22 Thread Brian Rogers
For some cards, ir_dev->props and ir_dev->raw are both NULL. These cards are
using built-in IR decoding instead of raw, and can't easily be made to switch
protocols.

So upon reading /sys/class/rc/rc?/protocols on such a card, return 'builtin' as
the supported and enabled protocol. Return -EINVAL on any attempts to change
the protocol. And most important of all, don't crash.

Signed-off-by: Brian Rogers 
Acked-by: Jarod Wilson 
---
 drivers/media/IR/ir-sysfs.c |   17 +++--
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index 96dafc4..46d4246 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -67,13 +67,14 @@ static ssize_t show_protocols(struct device *d,
char *tmp = buf;
int i;
 
-   if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+   if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
enabled = ir_dev->rc_tab.ir_type;
allowed = ir_dev->props->allowed_protos;
-   } else {
+   } else if (ir_dev->raw) {
enabled = ir_dev->raw->enabled_protocols;
allowed = ir_raw_get_allowed_protocols();
-   }
+   } else
+   return sprintf(tmp, "[builtin]\n");
 
IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n",
   (long long)allowed,
@@ -121,10 +122,14 @@ static ssize_t store_protocols(struct device *d,
int rc, i, count = 0;
unsigned long flags;
 
-   if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
+   if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
type = ir_dev->rc_tab.ir_type;
-   else
+   else if (ir_dev->raw)
type = ir_dev->raw->enabled_protocols;
+   else {
+   IR_dprintk(1, "Protocol switching not supported\n");
+   return -EINVAL;
+   }
 
while ((tmp = strsep((char **) &data, " \n")) != NULL) {
if (!*tmp)
@@ -185,7 +190,7 @@ static ssize_t store_protocols(struct device *d,
}
}
 
-   if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+   if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
spin_lock_irqsave(&ir_dev->rc_tab.lock, flags);
ir_dev->rc_tab.ir_type = type;
spin_unlock_irqrestore(&ir_dev->rc_tab.lock, flags);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface

2010-09-15 Thread Jarod Wilson
On Wed, Sep 15, 2010 at 05:57:10AM -0700, Brian Rogers wrote:
>  On 09/08/2010 07:16 AM, Jarod Wilson wrote:
> >On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:
> >>ir_dev->raw is also null. If I check these pointers before using
> >>them, and bail out if both are null, then I get a working lircd, but
> >>of course the file /sys/devices/virtual/rc/rc0/protocols no longer
> >>does anything. On 2.6.35.4, the system never creates the
> >>/sys/class/rc/rc0/current_protocol file. Is it the case that the
> >>'protocols' file should never appear, because my card can't support
> >>this feature?
> >Hm... So protocols is indeed intended for hardware that handles raw IR, as
> >its a list of raw IR decoders available/enabled/disabled for the receiver.
> >But some devices that do onboard decoding and deal with scancodes still
> >need to support changing protocols, as they can be told "decode rc5" or
> >"decode nec", etc... My memory is currently foggy on how it was exactly
> >that it was supposed to be donee though. :) (Yet another reason I really
> >need to poke at the imon driver code again).
> 
> How about the attached patch? Does this look like a reasonable
> solution for 2.6.36?
> 
> Brian
> 

> From 7937051c5e2c8b5b0410172d48e62d54bd1906ee Mon Sep 17 00:00:00 2001
> From: Brian Rogers 
> Date: Wed, 8 Sep 2010 05:33:34 -0700
> Subject: [PATCH] ir-core: Fix null dereferences in the protocols sysfs 
> interface
> 
> For some cards, ir_dev->props and ir_dev->raw are both NULL. These cards are
> using built-in IR decoding instead of raw, and can't easily be made to switch
> protocols.
> 
> So upon reading /sys/class/rc/rc?/protocols on such a card, return 'builtin' 
> as
> the supported and enabled protocol. Return -EINVAL on any attempts to change
> the protocol. And most important of all, don't crash.
> 
> Signed-off-by: Brian Rogers 
> ---
>  drivers/media/IR/ir-sysfs.c |   17 +++--
>  1 files changed, 11 insertions(+), 6 deletions(-)

Yeah, this looks pretty sane for 2.6.36, would just be a short-lived panic
preventer until David's interface changes get merged after 2.6.37-rc1.

Acked-by: Jarod Wilson 


-- 
Jarod Wilson
ja...@redhat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ir-core: Fix null dereferences in the protocols sysfs interface

2010-09-15 Thread Brian Rogers

 On 09/08/2010 07:16 AM, Jarod Wilson wrote:

On Wed, Sep 08, 2010 at 07:04:03AM -0700, Brian Rogers wrote:

ir_dev->raw is also null. If I check these pointers before using
them, and bail out if both are null, then I get a working lircd, but
of course the file /sys/devices/virtual/rc/rc0/protocols no longer
does anything. On 2.6.35.4, the system never creates the
/sys/class/rc/rc0/current_protocol file. Is it the case that the
'protocols' file should never appear, because my card can't support
this feature?

Hm... So protocols is indeed intended for hardware that handles raw IR, as
its a list of raw IR decoders available/enabled/disabled for the receiver.
But some devices that do onboard decoding and deal with scancodes still
need to support changing protocols, as they can be told "decode rc5" or
"decode nec", etc... My memory is currently foggy on how it was exactly
that it was supposed to be donee though. :) (Yet another reason I really
need to poke at the imon driver code again).


How about the attached patch? Does this look like a reasonable solution 
for 2.6.36?


Brian

>From 7937051c5e2c8b5b0410172d48e62d54bd1906ee Mon Sep 17 00:00:00 2001
From: Brian Rogers 
Date: Wed, 8 Sep 2010 05:33:34 -0700
Subject: [PATCH] ir-core: Fix null dereferences in the protocols sysfs interface

For some cards, ir_dev->props and ir_dev->raw are both NULL. These cards are
using built-in IR decoding instead of raw, and can't easily be made to switch
protocols.

So upon reading /sys/class/rc/rc?/protocols on such a card, return 'builtin' as
the supported and enabled protocol. Return -EINVAL on any attempts to change
the protocol. And most important of all, don't crash.

Signed-off-by: Brian Rogers 
---
 drivers/media/IR/ir-sysfs.c |   17 +++--
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index 96dafc4..46d4246 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -67,13 +67,14 @@ static ssize_t show_protocols(struct device *d,
 	char *tmp = buf;
 	int i;
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
 		enabled = ir_dev->rc_tab.ir_type;
 		allowed = ir_dev->props->allowed_protos;
-	} else {
+	} else if (ir_dev->raw) {
 		enabled = ir_dev->raw->enabled_protocols;
 		allowed = ir_raw_get_allowed_protocols();
-	}
+	} else
+		return sprintf(tmp, "[builtin]\n");
 
 	IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n",
 		   (long long)allowed,
@@ -121,10 +122,14 @@ static ssize_t store_protocols(struct device *d,
 	int rc, i, count = 0;
 	unsigned long flags;
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE)
 		type = ir_dev->rc_tab.ir_type;
-	else
+	else if (ir_dev->raw)
 		type = ir_dev->raw->enabled_protocols;
+	else {
+		IR_dprintk(1, "Protocol switching not supported\n");
+		return -EINVAL;
+	}
 
 	while ((tmp = strsep((char **) &data, " \n")) != NULL) {
 		if (!*tmp)
@@ -185,7 +190,7 @@ static ssize_t store_protocols(struct device *d,
 		}
 	}
 
-	if (ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
+	if (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_SCANCODE) {
 		spin_lock_irqsave(&ir_dev->rc_tab.lock, flags);
 		ir_dev->rc_tab.ir_type = type;
 		spin_unlock_irqrestore(&ir_dev->rc_tab.lock, flags);
-- 
1.7.1