For some reason we endedup allocating sdw_stream_runtime for every cpu dai, this has two issues. 1. we never set snd_soc_dai_set_stream for non soundwire dai, which means there is no way that we can free this, resulting in memory leak 2. startup and shutdown callbacks can be called without hw_params callback called. This combination results in memory leak because machine driver sruntime array pointer is only set in hw_params callback.
Fix this by 1. adding a helper function to get sdw_runtime for substream which can be used by shutdown callback to get hold of sruntime to free. 2. only allocate sdw_runtime for soundwire dais.
Fixes: d32bac9cb09c ("ASoC: qcom: Add helper for allocating Soundwire stream runtime") Cc: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Cc: Stable@vger.kernel.org Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@oss.qualcomm.com --- sound/soc/qcom/sc7280.c | 2 +- sound/soc/qcom/sc8280xp.c | 2 +- sound/soc/qcom/sdw.c | 105 +++++++++++++++++++++----------------- sound/soc/qcom/sdw.h | 1 + sound/soc/qcom/sm8250.c | 2 +- sound/soc/qcom/x1e80100.c | 2 +- 6 files changed, 64 insertions(+), 50 deletions(-)
diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c index af412bd0c89f..c444dae563c7 100644 --- a/sound/soc/qcom/sc7280.c +++ b/sound/soc/qcom/sc7280.c @@ -317,7 +317,7 @@ static void sc7280_snd_shutdown(struct snd_pcm_substream *substream) struct snd_soc_card *card = rtd->card; struct sc7280_snd_data *data = snd_soc_card_get_drvdata(card); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); - struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id]; + struct sdw_stream_runtime *sruntime = qcom_snd_sdw_get_stream(substream);
switch (cpu_dai->id) { case MI2S_PRIMARY: diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c index 78e327bc2f07..9ba536dff667 100644 --- a/sound/soc/qcom/sc8280xp.c +++ b/sound/soc/qcom/sc8280xp.c @@ -73,7 +73,7 @@ static void sc8280xp_snd_shutdown(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); struct sc8280xp_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card); - struct sdw_stream_runtime *sruntime = pdata->sruntime[cpu_dai->id]; + struct sdw_stream_runtime *sruntime = qcom_snd_sdw_get_stream(substream);
pdata->sruntime[cpu_dai->id] = NULL; sdw_release_stream(sruntime); diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c index 7d7981d4295b..7b2cae92c812 100644 --- a/sound/soc/qcom/sdw.c +++ b/sound/soc/qcom/sdw.c @@ -7,6 +7,37 @@ #include <sound/soc.h> #include "sdw.h"
+static bool qcom_snd_is_sdw_dai(int id) +{ + switch (id) { + case WSA_CODEC_DMA_RX_0: + case WSA_CODEC_DMA_TX_0: + case WSA_CODEC_DMA_RX_1: + case WSA_CODEC_DMA_TX_1: + case WSA_CODEC_DMA_TX_2: + case RX_CODEC_DMA_RX_0: + case TX_CODEC_DMA_TX_0: + case RX_CODEC_DMA_RX_1: + case TX_CODEC_DMA_TX_1: + case RX_CODEC_DMA_RX_2: + case TX_CODEC_DMA_TX_2: + case RX_CODEC_DMA_RX_3: + case TX_CODEC_DMA_TX_3: + case RX_CODEC_DMA_RX_4: + case TX_CODEC_DMA_TX_4: + case RX_CODEC_DMA_RX_5: + case TX_CODEC_DMA_TX_5: + case RX_CODEC_DMA_RX_6: + case RX_CODEC_DMA_RX_7: + case SLIMBUS_0_RX...SLIMBUS_6_TX: + return true; + default: + break; + } + + return false; +} + /** * qcom_snd_sdw_startup() - Helper to start Soundwire stream for SoC audio card * @substream: The PCM substream from audio, as passed to snd_soc_ops->startup() @@ -29,6 +60,9 @@ int qcom_snd_sdw_startup(struct snd_pcm_substream *substream) u32 rx_ch_cnt = 0, tx_ch_cnt = 0; int ret, i, j;
+ if (!qcom_snd_is_sdw_dai(cpu_dai->id)) + return 0; + sruntime = sdw_alloc_stream(cpu_dai->name, SDW_STREAM_PCM); if (!sruntime) return -ENOMEM; @@ -89,19 +123,8 @@ int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream, if (!sruntime) return 0;
- switch (cpu_dai->id) { - case WSA_CODEC_DMA_RX_0: - case WSA_CODEC_DMA_RX_1: - case RX_CODEC_DMA_RX_0: - case RX_CODEC_DMA_RX_1: - case TX_CODEC_DMA_TX_0: - case TX_CODEC_DMA_TX_1: - case TX_CODEC_DMA_TX_2: - case TX_CODEC_DMA_TX_3: - break; - default: + if (!qcom_snd_is_sdw_dai(cpu_dai->id)) return 0; - }
if (*stream_prepared) return 0; @@ -129,9 +152,7 @@ int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream, } EXPORT_SYMBOL_GPL(qcom_snd_sdw_prepare);
-int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct sdw_stream_runtime **psruntime) +struct sdw_stream_runtime *qcom_snd_sdw_get_stream(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai *codec_dai; @@ -139,21 +160,23 @@ int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream, struct sdw_stream_runtime *sruntime; int i;
- switch (cpu_dai->id) { - case WSA_CODEC_DMA_RX_0: - case RX_CODEC_DMA_RX_0: - case RX_CODEC_DMA_RX_1: - case TX_CODEC_DMA_TX_0: - case TX_CODEC_DMA_TX_1: - case TX_CODEC_DMA_TX_2: - case TX_CODEC_DMA_TX_3: - for_each_rtd_codec_dais(rtd, i, codec_dai) { - sruntime = snd_soc_dai_get_stream(codec_dai, substream->stream); - if (sruntime != ERR_PTR(-ENOTSUPP)) - *psruntime = sruntime; - } - break; + if (!qcom_snd_is_sdw_dai(cpu_dai->id)) + return NULL; + + for_each_rtd_codec_dais(rtd, i, codec_dai) { + sruntime = snd_soc_dai_get_stream(codec_dai, substream->stream); + if (sruntime != ERR_PTR(-ENOTSUPP)) + return sruntime; } + return NULL; +} +EXPORT_SYMBOL_GPL(qcom_snd_sdw_get_stream); + +int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct sdw_stream_runtime **psruntime) +{ + *psruntime = qcom_snd_sdw_get_stream(substream);
return 0;
@@ -166,23 +189,13 @@ int qcom_snd_sdw_hw_free(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
- switch (cpu_dai->id) { - case WSA_CODEC_DMA_RX_0: - case WSA_CODEC_DMA_RX_1: - case RX_CODEC_DMA_RX_0: - case RX_CODEC_DMA_RX_1: - case TX_CODEC_DMA_TX_0: - case TX_CODEC_DMA_TX_1: - case TX_CODEC_DMA_TX_2: - case TX_CODEC_DMA_TX_3: - if (sruntime && *stream_prepared) { - sdw_disable_stream(sruntime); - sdw_deprepare_stream(sruntime); - *stream_prepared = false; - } - break; - default: - break; + if (!qcom_snd_is_sdw_dai(cpu_dai->id)) + return 0; + + if (sruntime && *stream_prepared) { + sdw_disable_stream(sruntime); + sdw_deprepare_stream(sruntime); + *stream_prepared = false; }
return 0; diff --git a/sound/soc/qcom/sdw.h b/sound/soc/qcom/sdw.h index 392e3455f1b1..b8bc5beb0522 100644 --- a/sound/soc/qcom/sdw.h +++ b/sound/soc/qcom/sdw.h @@ -10,6 +10,7 @@ int qcom_snd_sdw_startup(struct snd_pcm_substream *substream); int qcom_snd_sdw_prepare(struct snd_pcm_substream *substream, struct sdw_stream_runtime *runtime, bool *stream_prepared); +struct sdw_stream_runtime *qcom_snd_sdw_get_stream(struct snd_pcm_substream *stream); int qcom_snd_sdw_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct sdw_stream_runtime **psruntime); diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c index f5b75a06e5bd..ce5b0059207f 100644 --- a/sound/soc/qcom/sm8250.c +++ b/sound/soc/qcom/sm8250.c @@ -117,7 +117,7 @@ static void sm8250_snd_shutdown(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); struct sm8250_snd_data *data = snd_soc_card_get_drvdata(rtd->card); - struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id]; + struct sdw_stream_runtime *sruntime = qcom_snd_sdw_get_stream(substream);
data->sruntime[cpu_dai->id] = NULL; sdw_release_stream(sruntime); diff --git a/sound/soc/qcom/x1e80100.c b/sound/soc/qcom/x1e80100.c index 444f2162889f..2e3599516aa2 100644 --- a/sound/soc/qcom/x1e80100.c +++ b/sound/soc/qcom/x1e80100.c @@ -55,7 +55,7 @@ static void x1e80100_snd_shutdown(struct snd_pcm_substream *substream) struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0); struct x1e80100_snd_data *data = snd_soc_card_get_drvdata(rtd->card); - struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id]; + struct sdw_stream_runtime *sruntime = qcom_snd_sdw_get_stream(substream);
data->sruntime[cpu_dai->id] = NULL; sdw_release_stream(sruntime);
Hi Srini,
On the Thinkpad X13s, with this patchset applied, we end up seeing a NULL pointer dereference:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020 Mem abort info: ESR = 0x0000000096000004 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages, 48-bit VAs, pgdp=000000010abfe000 pgd=0000000000000000, p4d=0000000000000000 SMP pdr_interface(E) crc8(E) phy_qcom_qmp_pcie(E) icc_osm_l3(E) gpio_sbu_mux(E) qcom_wdt(E) typec(E) qcom_pdr_msg(E) qmi_helpers(E) smp2p(E) rpmsg_core(E) fixed(E) gpio_keys(E) qnoc_sc8280xp(E) pwm_bl(E) smem(E) efivarfs(E) CPU: 5 UID: 111 PID: 1501 Comm: wireplumber Tainted: G E 6.17.4 #2 PREEMPTLAZY Tainted: [E]=UNSIGNED_MODULE Hardware name: LENOVO 21BX0015US/21BX0015US, BIOS N3HET94W (1.66 ) 09/15/2025 pstate: 60401005 (nZCv daif +PAN -UAO -TCO -DIT +SSBS BTYPE=--) pc : sdw_stream_add_slave+0x4c/0x440 [soundwire_bus] lr : sdw_stream_add_slave+0x4c/0x440 [soundwire_bus] sp : ffff80008bc2b250 x29: ffff80008bc2b250 x28: ffff0000a56b2f88 x27: 0000000000000000 x26: 0000000000000000 x25: ffff0000a301b000 x24: 0000000000000000 x23: ffff0000e8ce0280 x22: 0000000000000000 x21: 0000000000000000 x20: ffff80008bc2b300 x19: ffff0000a305a880 x18: 0000000000000000 x17: 0000000000000000 x16: ffffb9859eb15c48 x15: 0000000000000000 x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 0000000000000000 x9 : ffffb985391c0614 x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff0000a56b2fd0 x5 : ffff0000a56b2f80 x4 : 0000000000000000 x3 : 0000000000000000 x2 : ffff0000eb005000 x1 : 0000000000000000 x0 : ffff00011de2c890 Call trace: (P) wcd938x_sdw_hw_params+0xa8/0x200 [snd_soc_wcd938x_sdw] wcd938x_codec_hw_params+0x48/0x88 [snd_soc_wcd938x] snd_soc_dai_hw_params+0x44/0x90 [snd_soc_core] __soc_pcm_hw_params+0x230/0x620 [snd_soc_core] dpcm_be_dai_hw_params+0x260/0x888 [snd_soc_core] dpcm_fe_dai_hw_params+0xc4/0x3b0 [snd_soc_core] snd_pcm_hw_params+0x180/0x468 [snd_pcm] snd_pcm_common_ioctl+0xc00/0x18b8 [snd_pcm] snd_pcm_ioctl+0x38/0x60 [snd_pcm] __arm64_sys_ioctl+0xac/0x108 invoke_syscall.constprop.0+0x64/0xe8 el0_svc_common.constprop.0+0xc0/0xe8 do_el0_svc+0x24/0x38 el0_svc+0x3c/0x170 el0t_64_sync_handler+0xa0/0xf0 el0t_64_sync+0x198/0x1a0 Code: f9418c00 b9006fe3 91004000 97ffe306 (f8420f43) ---[ end trace 0000000000000000 ]---
-- steev
On 10/22/25 1:34 AM, Steev Klimaszewski wrote:
Hi Srini,
On the Thinkpad X13s, with this patchset applied, we end up seeing a NULL pointer dereference:
Thanks Steev, I think I know the issue, There was a silly typo in 3/4 patch. Could you please try this change, I will send this in v3 anyway;
-------------------------->cut<------------------------ diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c index 16bf09db29f5..6576b47a4c8c 100644 --- a/sound/soc/qcom/sdw.c +++ b/sound/soc/qcom/sdw.c @@ -31,6 +31,7 @@ static bool qcom_snd_is_sdw_dai(int id) case RX_CODEC_DMA_RX_6: case RX_CODEC_DMA_RX_7: case SLIMBUS_0_RX...SLIMBUS_6_TX: + return true; default: break; }
-------------------------->cut<------------------------
thanks, Srini> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020
Mem abort info: ESR = 0x0000000096000004 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages, 48-bit VAs, pgdp=000000010abfe000 pgd=0000000000000000, p4d=0000000000000000 SMP pdr_interface(E) crc8(E) phy_qcom_qmp_pcie(E) icc_osm_l3(E) gpio_sbu_mux(E) qcom_wdt(E) typec(E) qcom_pdr_msg(E) qmi_helpers(E) smp2p(E) rpmsg_core(E) fixed(E) gpio_keys(E) qnoc_sc8280xp(E) pwm_bl(E) smem(E) efivarfs(E) CPU: 5 UID: 111 PID: 1501 Comm: wireplumber Tainted: G E 6.17.4 #2 PREEMPTLAZY Tainted: [E]=UNSIGNED_MODULE Hardware name: LENOVO 21BX0015US/21BX0015US, BIOS N3HET94W (1.66 ) 09/15/2025 pstate: 60401005 (nZCv daif +PAN -UAO -TCO -DIT +SSBS BTYPE=--) pc : sdw_stream_add_slave+0x4c/0x440 [soundwire_bus] lr : sdw_stream_add_slave+0x4c/0x440 [soundwire_bus] sp : ffff80008bc2b250 x29: ffff80008bc2b250 x28: ffff0000a56b2f88 x27: 0000000000000000 x26: 0000000000000000 x25: ffff0000a301b000 x24: 0000000000000000 x23: ffff0000e8ce0280 x22: 0000000000000000 x21: 0000000000000000 x20: ffff80008bc2b300 x19: ffff0000a305a880 x18: 0000000000000000 x17: 0000000000000000 x16: ffffb9859eb15c48 x15: 0000000000000000 x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000 x11: 0000000000000000 x10: 0000000000000000 x9 : ffffb985391c0614 x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff0000a56b2fd0 x5 : ffff0000a56b2f80 x4 : 0000000000000000 x3 : 0000000000000000 x2 : ffff0000eb005000 x1 : 0000000000000000 x0 : ffff00011de2c890 Call trace: (P) wcd938x_sdw_hw_params+0xa8/0x200 [snd_soc_wcd938x_sdw] wcd938x_codec_hw_params+0x48/0x88 [snd_soc_wcd938x] snd_soc_dai_hw_params+0x44/0x90 [snd_soc_core] __soc_pcm_hw_params+0x230/0x620 [snd_soc_core] dpcm_be_dai_hw_params+0x260/0x888 [snd_soc_core] dpcm_fe_dai_hw_params+0xc4/0x3b0 [snd_soc_core] snd_pcm_hw_params+0x180/0x468 [snd_pcm] snd_pcm_common_ioctl+0xc00/0x18b8 [snd_pcm] snd_pcm_ioctl+0x38/0x60 [snd_pcm] __arm64_sys_ioctl+0xac/0x108 invoke_syscall.constprop.0+0x64/0xe8 el0_svc_common.constprop.0+0xc0/0xe8 do_el0_svc+0x24/0x38 el0_svc+0x3c/0x170 el0t_64_sync_handler+0xa0/0xf0 el0t_64_sync+0x198/0x1a0 Code: f9418c00 b9006fe3 91004000 97ffe306 (f8420f43) ---[ end trace 0000000000000000 ]---
-- steev
linux-stable-mirror@lists.linaro.org