On Thursday, April 25, 2013 10:47:49 AM Aaron Lu wrote: > acpi_video_bus_get_devices may fail due to some video output device > doesn't have the _ADR method, and in this case, the error processing is > to simply free the video structure in acpi_video_bus_add, while leaving > those already registered video output devices in the wild, which means > for some video output device, we have already registered a backlight > interface and installed a notification handler for it. So it can happen > when user is using this system, on hotkey pressing, the notification > handler will send a keycode through a non-existing input device, causing > kernel freeze. > > To solve this problem, we free all those already registered video output > devices once something goes wrong in acpi_video_bus_get_devices so that > no wild backlight interfaces and notification handlers exist. > > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=51731 > Reported-bisected-and-tested-by: <i-tek@xxxxxx> > Signed-off-by: Aaron Lu <aaron.lu@xxxxxxxxx> Applied. Thanks, Rafael > --- > drivers/acpi/video.c | 143 ++++++++++++++++++++++++--------------------------- > 1 file changed, 66 insertions(+), 77 deletions(-) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index ed192e5..c3932d0 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -167,7 +167,8 @@ struct acpi_video_device_flags { > u8 dvi:1; > u8 bios:1; > u8 unknown:1; > - u8 reserved:2; > + u8 notify:1; > + u8 reserved:1; > }; > > struct acpi_video_device_cap { > @@ -1074,53 +1075,51 @@ acpi_video_bus_get_one_device(struct acpi_device *device, > struct acpi_video_device *data; > struct acpi_video_device_attrib* attribute; > > - if (!device || !video) > - return -EINVAL; > - > status = > acpi_evaluate_integer(device->handle, "_ADR", NULL, &device_id); > - if (ACPI_SUCCESS(status)) { > - > - data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL); > - if (!data) > - return -ENOMEM; > - > - strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME); > - strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS); > - device->driver_data = data; > - > - data->device_id = device_id; > - data->video = video; > - data->dev = device; > + /* Some device omits _ADR, we skip them instead of fail */ > + if (ACPI_FAILURE(status)) > + return 0; > > - attribute = acpi_video_get_device_attr(video, device_id); > + data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > > - if((attribute != NULL) && attribute->device_id_scheme) { > - switch (attribute->display_type) { > - case ACPI_VIDEO_DISPLAY_CRT: > - data->flags.crt = 1; > - break; > - case ACPI_VIDEO_DISPLAY_TV: > - data->flags.tvout = 1; > - break; > - case ACPI_VIDEO_DISPLAY_DVI: > - data->flags.dvi = 1; > - break; > - case ACPI_VIDEO_DISPLAY_LCD: > - data->flags.lcd = 1; > - break; > - default: > - data->flags.unknown = 1; > - break; > - } > - if(attribute->bios_can_detect) > - data->flags.bios = 1; > - } else { > - /* Check for legacy IDs */ > - device_type = acpi_video_get_device_type(video, > - device_id); > - /* Ignore bits 16 and 18-20 */ > - switch (device_type & 0xffe2ffff) { > + strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME); > + strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS); > + device->driver_data = data; > + > + data->device_id = device_id; > + data->video = video; > + data->dev = device; > + > + attribute = acpi_video_get_device_attr(video, device_id); > + > + if((attribute != NULL) && attribute->device_id_scheme) { > + switch (attribute->display_type) { > + case ACPI_VIDEO_DISPLAY_CRT: > + data->flags.crt = 1; > + break; > + case ACPI_VIDEO_DISPLAY_TV: > + data->flags.tvout = 1; > + break; > + case ACPI_VIDEO_DISPLAY_DVI: > + data->flags.dvi = 1; > + break; > + case ACPI_VIDEO_DISPLAY_LCD: > + data->flags.lcd = 1; > + break; > + default: > + data->flags.unknown = 1; > + break; > + } > + if(attribute->bios_can_detect) > + data->flags.bios = 1; > + } else { > + /* Check for legacy IDs */ > + device_type = acpi_video_get_device_type(video, device_id); > + /* Ignore bits 16 and 18-20 */ > + switch (device_type & 0xffe2ffff) { > case ACPI_VIDEO_DISPLAY_LEGACY_MONITOR: > data->flags.crt = 1; > break; > @@ -1132,34 +1131,24 @@ acpi_video_bus_get_one_device(struct acpi_device *device, > break; > default: > data->flags.unknown = 1; > - } > } > + } > > - acpi_video_device_bind(video, data); > - acpi_video_device_find_cap(data); > - > - status = acpi_install_notify_handler(device->handle, > - ACPI_DEVICE_NOTIFY, > - acpi_video_device_notify, > - data); > - if (ACPI_FAILURE(status)) { > - printk(KERN_ERR PREFIX > - "Error installing notify handler\n"); > - if(data->brightness) > - kfree(data->brightness->levels); > - kfree(data->brightness); > - kfree(data); > - return -ENODEV; > - } > + acpi_video_device_bind(video, data); > + acpi_video_device_find_cap(data); > > - mutex_lock(&video->device_list_lock); > - list_add_tail(&data->entry, &video->video_device_list); > - mutex_unlock(&video->device_list_lock); > + status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, > + acpi_video_device_notify, data); > + if (ACPI_FAILURE(status)) > + dev_err(&device->dev, "Error installing notify handler\n"); > + else > + data->flags.notify = 1; > > - return 0; > - } > + mutex_lock(&video->device_list_lock); > + list_add_tail(&data->entry, &video->video_device_list); > + mutex_unlock(&video->device_list_lock); > > - return -ENOENT; > + return status; > } > > /* > @@ -1452,9 +1441,8 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video, > > status = acpi_video_bus_get_one_device(dev, video); > if (status) { > - printk(KERN_WARNING PREFIX > - "Can't attach device\n"); > - continue; > + dev_err(&dev->dev, "Can't attach device\n"); > + break; > } > } > return status; > @@ -1467,13 +1455,14 @@ static int acpi_video_bus_put_one_device(struct acpi_video_device *device) > if (!device || !device->video) > return -ENOENT; > > - status = acpi_remove_notify_handler(device->dev->handle, > - ACPI_DEVICE_NOTIFY, > - acpi_video_device_notify); > - if (ACPI_FAILURE(status)) { > - printk(KERN_WARNING PREFIX > - "Can't remove video notify handler\n"); > + if (device->flags.notify) { > + status = acpi_remove_notify_handler(device->dev->handle, > + ACPI_DEVICE_NOTIFY, acpi_video_device_notify); > + if (ACPI_FAILURE(status)) > + dev_err(&device->dev->dev, > + "Can't remove video notify handler\n"); > } > + > if (device->backlight) { > backlight_device_unregister(device->backlight); > device->backlight = NULL; > @@ -1755,7 +1744,7 @@ static int acpi_video_bus_add(struct acpi_device *device) > > error = acpi_video_bus_get_devices(video, device); > if (error) > - goto err_free_video; > + goto err_put_video; > > video->input = input = input_allocate_device(); > if (!input) { > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
