| Message ID | 20120907064740.GB12028@elgon.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Fri, 07 September 2012 Dan Carpenter <dan.carpenter@oracle.com> wrote: > Smatch complains that the NULL checking in this function is not > consistent and could lead to a NULL dereference. The comments say that > we should return here if rc_dev is NULL so I've changed the test to > match the comment. Good catch! Currently thanks to HID mutex around probe()/remove() we should never be able to see rc_dev being NULL here. Once the probe()/remove() mutex gets tied to hw_start()/hw_stop() or otherwise changed to allow drivers to chat with device during probe() ->rc_dev might be NULL in picolcd_raw_cir(). Reviewed-by: Bruno Prémont <bonbons@linux-vserver.org> Jiri, please apply to picolcd branch. Thanks, Bruno > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > Only needed in linux-next. This is a static checker fix and I don't > have the hardware to test it. Please review carefully. > > diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c > index 14c5ce0..13ca919 100644 > --- a/drivers/hid/hid-picolcd_cir.c > +++ b/drivers/hid/hid-picolcd_cir.c > @@ -51,7 +51,7 @@ int picolcd_raw_cir(struct picolcd_data *data, > > /* ignore if rc_dev is NULL or status is shunned */ > spin_lock_irqsave(&data->lock, flags); > - if (data->rc_dev && (data->status & PICOLCD_CIR_SHUN)) { > + if (!data->rc_dev || (data->status & PICOLCD_CIR_SHUN)) { > spin_unlock_irqrestore(&data->lock, flags); > return 1; > } -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 7 Sep 2012, Bruno Prémont wrote: > > Smatch complains that the NULL checking in this function is not > > consistent and could lead to a NULL dereference. The comments say that > > we should return here if rc_dev is NULL so I've changed the test to > > match the comment. > > Good catch! Currently thanks to HID mutex around probe()/remove() > we should never be able to see rc_dev being NULL here. > > > Once the probe()/remove() mutex gets tied to hw_start()/hw_stop() or > otherwise changed to allow drivers to chat with device during probe() > ->rc_dev might be NULL in picolcd_raw_cir(). > > > Reviewed-by: Bruno Prémont <bonbons@linux-vserver.org> Applied, thanks Dan, thanks Bruno.
diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c index 14c5ce0..13ca919 100644 --- a/drivers/hid/hid-picolcd_cir.c +++ b/drivers/hid/hid-picolcd_cir.c @@ -51,7 +51,7 @@ int picolcd_raw_cir(struct picolcd_data *data, /* ignore if rc_dev is NULL or status is shunned */ spin_lock_irqsave(&data->lock, flags); - if (data->rc_dev && (data->status & PICOLCD_CIR_SHUN)) { + if (!data->rc_dev || (data->status & PICOLCD_CIR_SHUN)) { spin_unlock_irqrestore(&data->lock, flags); return 1; }
Smatch complains that the NULL checking in this function is not consistent and could lead to a NULL dereference. The comments say that we should return here if rc_dev is NULL so I've changed the test to match the comment. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Only needed in linux-next. This is a static checker fix and I don't have the hardware to test it. Please review carefully. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html