Message ID | CA+GA0_sOQeQsaa1JFO3+ySqdLU6BNxrJRrHjqtheEuj60ZmwhA@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | soundwire: intel: fix possible crash when no device is detected (was Re: Crash in acpi_ns_validate_handle triggered by soundwire on Linux 5.10) | expand |
Thanks Marcin for the patch, much appreciated. > acpi_walk_namespace can return success without executing our > callback which initializes info->handle. > If the random value in this structure is a valid address (which > is on the stack, so it's quite possible), then nothing bad will > happen, because: > sdw_intel_scan_controller > -> acpi_bus_get_device > -> acpi_get_device_data > -> acpi_get_data_full > -> acpi_ns_validate_handle > will reject this handle. > > However, if the value from the stack doesn't point to a valid > address, we get this: > > BUG: kernel NULL pointer dereference, address: 0000000000000050 [...] > diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c > index cabdadb09a1b..bc8520eb385e 100644 > --- a/drivers/soundwire/intel_init.c > +++ b/drivers/soundwire/intel_init.c > @@ -405,11 +405,12 @@ int sdw_intel_acpi_scan(acpi_handle *parent_handle, > { > acpi_status status; > > + info->handle = NULL; > status = acpi_walk_namespace(ACPI_TYPE_DEVICE, > parent_handle, 1, > sdw_intel_acpi_cb, > NULL, info, NULL); > - if (ACPI_FAILURE(status)) > + if (ACPI_FAILURE(status) || info->handle == NULL) > return -ENODEV; > > return sdw_intel_scan_controller(info); It does seem like a required code pattern if I look at I2C and SPI. I had no idea. Maybe worth documenting? Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index cabdadb09a1b..bc8520eb385e 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -405,11 +405,12 @@ int sdw_intel_acpi_scan(acpi_handle *parent_handle, { acpi_status status; + info->handle = NULL; status = acpi_walk_namespace(ACPI_TYPE_DEVICE, parent_handle, 1, sdw_intel_acpi_cb, NULL, info, NULL); - if (ACPI_FAILURE(status)) + if (ACPI_FAILURE(status) || info->handle == NULL) return -ENODEV; return sdw_intel_scan_controller(info);