From 38c660433a0192d8c72391108791ee8c461cdf38 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Mon, 2 Dec 2024 11:39:23 +0000 Subject: [PATCH 1/2] ASoC: Intel: sof_sdw: Add space for a terminator into DAIs array The code uses the initialised member of the asoc_sdw_dailink struct to determine if a member of the array is in use. However in the case the array is completely full this will lead to an access 1 past the end of the array, expand the array by one entry to include a space for a terminator. Fixes: 27fd36aefa00 ("ASoC: Intel: sof-sdw: Add new code for parsing the snd_soc_acpi structs") Signed-off-by: Charles Keepax --- sound/soc/intel/boards/sof_sdw.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index b4e637f1cbf160..50b5b4867c9251 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -1108,8 +1108,12 @@ static int sof_card_dai_links_create(struct snd_soc_card *card) return ret; } - /* One per DAI link, worst case is a DAI link for every endpoint */ - sof_dais = kcalloc(num_ends, sizeof(*sof_dais), GFP_KERNEL); + /* + * One per DAI link, worst case is a DAI link for every endpoint, also + * add one additional to act as a terminator such that code can iterate + * until it hits an uninitialised DAI. + */ + sof_dais = kcalloc(num_ends + 1, sizeof(*sof_dais), GFP_KERNEL); if (!sof_dais) return -ENOMEM; From 7a3019ab496a14b70c58e7d2d8cad4818998867f Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Thu, 28 Nov 2024 10:57:49 +0000 Subject: [PATCH 2/2] soundwire: intel_auxdevice: Don't disable IRQs before removing children Currently the auxiliary device for the link disables IRQs before it calls sdw_bus_master_delete(). This has the side effect that none of the devices on the link can access their own registers whilst their remove functions run, because the IRQs are required for bus transactions to function. Obviously, devices should be able to access their own registers during disable to park the device suitably. It would appear the reason for the disabling of the IRQs is that the IRQ handler iterates through a linked list of all the links, once a link is removed the memory pointed at by this linked list is freed, but not removed from the linked_list itself. Add a list_del() for the linked list item, note whilst the list itself is contained in the intel_init portion of the code, the list remove needs to be attached to the auxiliary device for the link, since that owns the memory that the list points at. Locking is also required to ensure the IRQ handler runs before or after any additions/removals from the list. Signed-off-by: Charles Keepax --- drivers/soundwire/intel.h | 1 + drivers/soundwire/intel_auxdevice.c | 5 ++++- drivers/soundwire/intel_init.c | 16 ++++++++++++++++ include/linux/soundwire/sdw_intel.h | 1 + 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h index dddd293814418b..4df582ceaed1a3 100644 --- a/drivers/soundwire/intel.h +++ b/drivers/soundwire/intel.h @@ -45,6 +45,7 @@ struct sdw_intel_link_res { u32 link_mask; struct sdw_cdns *cdns; struct list_head list; + struct mutex *link_lock; /* lock protecting list */ struct hdac_bus *hbus; }; diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c index dee126f6d9d5a2..cb0e0d6ec18904 100644 --- a/drivers/soundwire/intel_auxdevice.c +++ b/drivers/soundwire/intel_auxdevice.c @@ -477,9 +477,12 @@ static void intel_link_remove(struct auxiliary_device *auxdev) if (!bus->prop.hw_disabled) { sdw_intel_debugfs_exit(sdw); cancel_delayed_work_sync(&cdns->attach_dwork); - sdw_cdns_enable_interrupt(cdns, false); } + sdw_bus_master_delete(bus); + + if (!bus->prop.hw_disabled) + sdw_cdns_enable_interrupt(cdns, false); } int intel_link_process_wakeen_event(struct auxiliary_device *auxdev) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 12e7a98f319f8c..db49ee38081513 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -28,6 +28,15 @@ static void intel_link_dev_release(struct device *dev) kfree(ldev); } +static void intel_link_list_del(void *data) +{ + struct sdw_intel_link_res *link = data; + + mutex_lock(link->link_lock); + list_del(&link->list); + mutex_unlock(link->link_lock); +} + /* alloc, init and add link devices */ static struct sdw_intel_link_dev *intel_link_dev_register(struct sdw_intel_res *res, struct sdw_intel_ctx *ctx, @@ -78,6 +87,7 @@ static struct sdw_intel_link_dev *intel_link_dev_register(struct sdw_intel_res * link->shim_vs = res->mmio_base + SDW_SHIM2_VS_BASE(link_id); link->shim_lock = res->eml_lock; } + link->link_lock = &ctx->link_lock; link->ops = res->ops; link->dev = res->dev; @@ -144,8 +154,10 @@ irqreturn_t sdw_intel_thread(int irq, void *dev_id) struct sdw_intel_ctx *ctx = dev_id; struct sdw_intel_link_res *link; + mutex_lock(&ctx->link_lock); list_for_each_entry(link, &ctx->link_list, list) sdw_cdns_irq(irq, link->cdns); + mutex_unlock(&ctx->link_lock); return IRQ_HANDLED; } @@ -209,6 +221,7 @@ static struct sdw_intel_ctx ctx->link_mask = res->link_mask; ctx->handle = res->handle; mutex_init(&ctx->shim_lock); + mutex_init(&ctx->link_lock); link_mask = ctx->link_mask; @@ -245,7 +258,10 @@ static struct sdw_intel_ctx i++; goto err; } + mutex_lock(&ctx->link_lock); list_add_tail(&link->list, &ctx->link_list); + mutex_unlock(&ctx->link_lock); + devm_add_action_or_reset(&ldev->auxdev.dev, intel_link_list_del, link); bus = &link->cdns->bus; /* Calculate number of slaves */ list_for_each(node, &bus->slaves) diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h index 580086417e4b0e..4444c99aead5f7 100644 --- a/include/linux/soundwire/sdw_intel.h +++ b/include/linux/soundwire/sdw_intel.h @@ -304,6 +304,7 @@ struct sdw_intel_ctx { acpi_handle handle; struct sdw_intel_link_dev **ldev; struct list_head link_list; + struct mutex link_lock; /* lock protecting link_list */ struct mutex shim_lock; /* lock for access to shared SHIM registers */ u32 shim_mask; u32 shim_base;