Potential initialization bug in leds-lp5523

Dan Carpenter dan.carpenter at oracle.com
Fri Apr 13 08:15:06 UTC 2012


On Wed, Apr 11, 2012 at 03:46:16PM -0500, Matt Renzelmann wrote:
> Hello,
> 
> I'm writing to report a potential bug in this driver.  I don't have a patch as
> I'm unsure the best way to fix it.  The error, which can lead to a kernel panic,
> is as follows:
> 
> - Suppose sysfs_create_group (line 860 in leds-lp5523.c in latest kernel) in
> lp5523_init_led fails.  The function will return early with an error code as a
> result.
> - Control returns to lp5523_probe, and breaks out of the loop to the fail3
> label.
> - In this failure case, the INIT_WORK macro is never called for this LED.  This
> normally executes in the same for() loop that calls lp5523_init_led, line 959,
> but because of the failure, the loop breaks execution early.
> - Execution continues with the cleanup loop:
> 
>         for (i = 0; i < chip->num_leds; i++) {
>                 led_classdev_unregister(&chip->leds[i].cdev);
>                 cancel_work_sync(&chip->leds[i].brightness_work);
>         }
> 
> - In this loop, we call led_classdev_unregister, which invokes
> led_brightness_set, and this in turn calls lp5523_set_brightness, which then
> calls schedule_work on the uninitialized work queue.  This can panic the kernel.
> 
> Can someone look at this and let me know if it's a genuine bug?  I found this
> using a tool we've developed and it'd be great to know if it's a false positive.
> It looks genuine to me but it'd be great if someone who knows what's going on
> could verify it :)  Please let me know if I can provide additional information.
> 

I don't think it's a bug.  The unregister loop only goes up to the
number of registered ->num_leds.  num_leds is only incremented if we
register successfully.

regards,
dan carpenter




More information about the devel mailing list