[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 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
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
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