Messages in this thread Patch in this message | | | Subject | Re: drivers/acpi/video.c: null pointer dereference | From | Len Brown <> | Date | 30 Mar 2005 22:26:55 -0500 |
| |
On Thu, 2005-03-24 at 15:37, Adrian Bunk wrote: > The Coverity checker found the following null pointer dereference in > drivers/acpi/video.c: > > <-- snip --> > > ... > static int > acpi_video_switch_output( > ... > { > ... > struct acpi_video_device *dev=NULL; > ... > list_for_each_safe(node, next, &video->video_device_list) { > struct acpi_video_device * dev = container_of(node, > struct acpi_video_device, entry); > ... > } > ... > switch (event) { > case ACPI_VIDEO_NOTIFY_CYCLE: > case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT: > acpi_video_device_set_state(dev, 0); > acpi_video_device_set_state(dev_next, 0x80000001); > break; > case ACPI_VIDEO_NOTIFY_PREV_OUTPUT: > acpi_video_device_set_state(dev, 0); > acpi_video_device_set_state(dev_prev, 0x80000001); > ... > > <-- snip --> > > > Two different variables of the same name within 40 lines of code are a > good indication that something's wrong... > > > The outer "dev" variable is never assigned any value different from > NULL. > > acpi_video_device_set_state dereferences this variable. > > > cu > Adrian
Looks like we should do this:
===== drivers/acpi/video.c 1.8 vs edited ===== --- 1.8/drivers/acpi/video.c 2005-01-06 02:06:20 -05:00 +++ edited/drivers/acpi/video.c 2005-03-24 15:44:33 -05:00 @@ -1585,7 +1585,7 @@ ACPI_FUNCTION_TRACE("acpi_video_switch_output"); list_for_each_safe(node, next, &video->video_device_list) { - struct acpi_video_device * dev = container_of(node, struct acpi_video_device, entry); + dev = container_of(node, struct acpi_video_device, entry); status = acpi_video_device_get_state(dev, &state); if (state & 0x2){ dev_next = container_of(node->next, struct acpi_video_device, entry); | |