Hi, On 1/18/21 2:58 PM, Rafael J. Wysocki wrote: > On Fri, Jan 15, 2021 at 10:59 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >> >> Set the acpi_device pointer which acpi_bus_get_device() >> returns-by-reference to NULL on error. >> >> We've recently had 2 cases where callers of acpi_bus_get_device() >> did not properly error check the return value, using the >> returned-by-reference acpi_device pointer blindly, set it to NULL >> so that this will lead to an immediate oops, rather then following >> a pointer to who knows what. >> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > This should fix the crash reported by Pierre-Louis, Ack, sounds good. > so let me apply it > instead of the two debug changes posted by me > (https://lore.kernel.org/linux-acpi/98e6ed8e-884e-adb4-a146-a66daefa94a7@xxxxxxxxxx/T/#md5add2fe554a30e3a929d87a66b435f4cc8bf628). Note we should still fix the USB case, my patch will make failure there more obvious, but the code can theoretically still dereference a NULL pointer in drivers/usb/core/usb-acpi.c. And we probably also want this change: --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1867,7 +1867,8 @@ static u32 acpi_scan_check_dep(acpi_hand * 2. ACPI nodes describing USB ports. * Still, checking for _HID catches more then just these cases ... */ - if (!acpi_has_method(handle, "_DEP") || !acpi_has_method(handle, "_HID")) + if (!acpi_has_method(handle, "_DEP") || acpi_has_method(handle, "_ADR") + || !acpi_has_method(handle, "_HID")) return 0; status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices); To reduce the amount of work we do checking _DEP-s. If you want I can submit patches for both ? Regards, Hans > > Pierre-Louis, can you please double check that the issue goes away > with this patch applied? > >> --- >> drivers/acpi/scan.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >> index c575c9b67f76..c53e88deee1d 100644 >> --- a/drivers/acpi/scan.c >> +++ b/drivers/acpi/scan.c >> @@ -585,6 +585,8 @@ static int acpi_get_device_data(acpi_handle handle, struct acpi_device **device, >> if (!device) >> return -EINVAL; >> >> + *device = NULL; >> + >> status = acpi_get_data_full(handle, acpi_scan_drop_device, >> (void **)device, callback); >> if (ACPI_FAILURE(status) || !*device) { >> -- >> 2.28.0 >> >
