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