Re: [PATCH] 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)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux