- Alsa-devel - mailman.alsa-project.org

[PATCH] ASoC: SOF: ipc3-topology: Prevent double freeing of ipc_control_data via load_bytes
by Peter Ujfalusi 12 Jul '22

12 Jul '22

We have sanity checks for byte controls and if any of the fail the locally allocated scontrol->ipc_control_data is freed up, but not set to NULL. On a rollback path of the error the higher level code will also try to free the scontrol->ipc_control_data which will eventually going to lead to memory corruption as double freeing memory is not a good thing. Fixes: b5cee8feb1d4 ("ASoC: SOF: topology: Make control parsing IPC agnostic") Reported-by: Seppo Ingalsuo <seppo.ingalsuo(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Seppo Ingalsuo <seppo.ingalsuo(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> --- sound/soc/sof/ipc3-topology.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c index 9448d5338423..b2cc046b9f60 100644 --- a/sound/soc/sof/ipc3-topology.c +++ b/sound/soc/sof/ipc3-topology.c @@ -1644,6 +1644,7 @@ static int sof_ipc3_control_load_bytes(struct snd_sof_dev *sdev, struct snd_sof_ return 0; err: kfree(scontrol->ipc_control_data); + scontrol->ipc_control_data = NULL; return ret; } -- 2.37.0

2 1

[PATCH 0/3] ASoC: SOF: Intel: Do not process IPC reply before firmware boot
by Peter Ujfalusi 12 Jul '22

12 Jul '22

Hi, By mistake a developer managed to create a 'corrupted' IPC4 firmware image which loaded fine to the DSP and after boot it sent an IPC reply before we would have received the FW_READY message. It turned out that the image was an IPC3 firmware and the IPC reply was the IPC3 FW_READY notification message which got understood as an IPC4 reply message due to the difference between the two IPC mechanism. This caused a NULL pointer dereference since the reply memory will be allocated after the FW_READY message. To make sure this will not bite again, skip any spurious reply messages before the FW_READY. Regards, Peter --- Peter Ujfalusi (3): ASoC: SOF: Intel: cnl: Do not process IPC reply before firmware boot ASoC: SOF: Intel: hda-ipc: Do not process IPC reply before firmware boot ASoC: SOF: Intel: mtl: Do not process IPC reply before firmware boot sound/soc/sof/intel/cnl.c | 37 +++++++++++++++++++++------------ sound/soc/sof/intel/hda-ipc.c | 39 ++++++++++++++++++++++------------- sound/soc/sof/intel/mtl.c | 20 +++++++++++------- 3 files changed, 62 insertions(+), 34 deletions(-) -- 2.37.0

2 4

alsalib assertion using plug+dmix
by GitHub issues - opened 12 Jul '22

12 Jul '22

alsa-project/alsa-plugins issue #46 was opened from npitre: The espeak program is crashing randomly here. I've been able to isolate the issue in a small test program demonstrating the crash 100% of the time, ending in: ``` alsa_quickcrash: pcm.c:3236: snd_pcm_area_copy: Assertion `src < dst || src >= dst + bytes' failed. Aborted (core dumped) ``` Here's the test code: ``` /* to compile: gcc -O2 -Wall -o alsa_quickcrash alsa_quickcrash.c -lasound */ #include <alsa/asoundlib.h> #include <stdio.h> #include <string.h> #define DEVICE "test_dummy" #define RATE 22050 #define CHANNELS 1 #define FORMAT SND_PCM_FORMAT_S16_LE #define BUF_MS 60 #define DO(x) if (x < 0) { fprintf(stderr, "%s failed\n", #x); exit(1); } int main() { unsigned int rate = RATE; snd_pcm_uframes_t frames = RATE * BUF_MS / 1000; snd_pcm_t *handle; snd_pcm_hw_params_t *params; DO(snd_pcm_hw_params_malloc(&params)); DO(snd_pcm_open(&handle, DEVICE, SND_PCM_STREAM_PLAYBACK, 0)); DO(snd_pcm_hw_params_any(handle, params)); DO(snd_pcm_hw_params_set_access(handle, params, SND_PCM_ACCESS_RW_INTERLEAVED)); DO(snd_pcm_hw_params_set_format(handle, params, FORMAT)); DO(snd_pcm_hw_params_set_rate_near(handle, params, &rate, 0)); DO(snd_pcm_hw_params_set_channels(handle, params, CHANNELS)); DO(snd_pcm_hw_params_set_buffer_size_near(handle, params, &frames)); DO(snd_pcm_hw_params(handle, params)); DO(snd_pcm_prepare(handle)); printf("PCM config dump:\n"); snd_output_t *output = NULL; DO(snd_output_stdio_attach(&output, stdout, 0)); DO(snd_pcm_dump(handle, output)); printf("\n"); printf("frames = %ld\n", (long)frames); short data[frames * CHANNELS]; memset(data, 0, sizeof(data)); printf("writing whole buffer\n"); DO(snd_pcm_writei(handle, data, frames)); printf("writing whole buffer\n"); DO(snd_pcm_writei(handle, data, frames)); printf("writing whole buffer minus 1 frame\n"); DO(snd_pcm_writei(handle, data, frames - 1)); printf("draining audio\n"); DO(snd_pcm_drain(handle)); printf("done\n"); DO(snd_pcm_close(handle)); return 0; } ``` Here's the corresponding .asoundrc content: ``` pcm.test_dummy { type plug slave.pcm "mix_dummy" } pcm.mix_dummy { type dmix ipc_key 1024 ipc_key_add_uid false ipc_perm 0666 slave { pcm "hw:0" period_size 1024 buffer_size 4096 format "S16_LE" rate 48000 } } ``` And here's the complete test output: ``` PCM config dump: Plug PCM: Rate conversion PCM (48000, sformat=S16_LE) Converter: linear-interpolation Protocol version: 10003 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 1 rate : 22050 exact rate : 22050 (22050/1) msbits : 16 buffer_size : 1411 period_size : 470 period_time : 21333 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 470 period_event : 0 start_threshold : 1 stop_threshold : 1411 silence_threshold: 0 silence_size : 0 boundary : 3177289537109884928 Slave: Route conversion PCM (sformat=S16_LE) Transformation table: 0 <- 0 1 <- 0 Its setup is: stream : PLAYBACK access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 1 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 3072 period_size : 1024 period_time : 21333 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 1024 period_event : 0 start_threshold : 2 stop_threshold : 3072 silence_threshold: 0 silence_size : 0 boundary : 6917529027641081856 Slave: Direct Stream Mixing PCM Its setup is: stream : PLAYBACK access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 3072 period_size : 1024 period_time : 21333 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 1024 period_event : 0 start_threshold : 2 stop_threshold : 3072 silence_threshold: 0 silence_size : 0 boundary : 6917529027641081856 Hardware PCM card 1 'HDA Intel PCH' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 4096 period_size : 1024 period_time : 21333 tstamp_mode : ENABLE tstamp_type : MONOTONIC period_step : 1 avail_min : 1024 period_event : 0 start_threshold : 1 stop_threshold : 0 silence_threshold: 0 silence_size : 0 boundary : 4611686018427387904 appl_ptr : 0 hw_ptr : 2734916609 frames = 1411 writing whole buffer writing whole buffer writing whole buffer minus 1 frame draining audio alsa_quickcrash: pcm.c:3236: snd_pcm_area_copy: Assertion `src < dst || src >= dst + bytes' failed. Aborted (core dumped) ``` Issue URL : https://github.com/alsa-project/alsa-plugins/issues/46 Repository URL: https://github.com/alsa-project/alsa-plugins

1 0

[PATCH] ASoC: soc-pcm: demote warnings on non-atomic BE connection
by Pierre-Louis Bossart 12 Jul '22

12 Jul '22

When an FE, typically non-atomic, is connected to an atomic BE, we force the BE as non-atomic. There's no reason to throw a warning, this is a perfectly fine configuration and a conversion that's required by-design. This removes the unconditional warnings such as [ 12.054213] iDisp1: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic [ 12.074693] iDisp2: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic [ 12.096612] iDisp3: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic [ 12.118637] iDisp4: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic [ 12.140660] dmic01: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic [ 12.147521] dmic16k: dpcm_be_connect: FE is nonatomic but BE is not, forcing BE as nonatomic and demotes them to dev_dbg(), as suggested in review comments. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Péter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> --- sound/soc/soc-pcm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index a827cc3c158a..5b99bf2dbd08 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1209,8 +1209,7 @@ static int dpcm_be_connect(struct snd_soc_pcm_runtime *fe, return -EINVAL; } if (fe_substream->pcm->nonatomic && !be_substream->pcm->nonatomic) { - dev_warn(be->dev, "%s: FE is nonatomic but BE is not, forcing BE as nonatomic\n", - __func__); + dev_dbg(be->dev, "FE is nonatomic but BE is not, forcing BE as nonatomic\n"); be_substream->pcm->nonatomic = 1; } -- 2.34.1

2 1

[PATCH] ASoC: SOF: topology: remove unused variable
by Peter Ujfalusi 12 Jul '22

12 Jul '22

From: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> 'ret' is never used. Remove it and return 0 instead. Signed-off-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> --- sound/soc/sof/topology.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 7e54eb1bf77b..9273a70fec25 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1419,7 +1419,6 @@ static int sof_widget_unload(struct snd_soc_component *scomp, struct soc_bytes_ext *sbe; struct snd_sof_dai *dai; struct soc_enum *se; - int ret = 0; int i; swidget = dobj->private; @@ -1480,7 +1479,7 @@ static int sof_widget_unload(struct snd_soc_component *scomp, list_del(&swidget->list); kfree(swidget); - return ret; + return 0; } /* -- 2.37.0

2 1

[PATCH 0/2] ASoC: SOF: Intel: hda: Use cold/purge boot after firmware crash
by Peter Ujfalusi 12 Jul '22

12 Jul '22

Hi, In case of a firmware crash we force the DSP to be powered down and rebooted. To make sure that the next boot is going to be clean, force the boot process to skip the IMR booting and re-download the firmware. Regards, Peter --- Peter Ujfalusi (2): ASoC: SOF: Intel: hda: Introduce skip_imr_boot flag ASoC: SOF: Intel: hda: Skip IMR boot after a firmware crash or boot failure sound/soc/sof/intel/hda-dsp.c | 12 ++++++++++++ sound/soc/sof/intel/hda-loader.c | 10 ++++++---- sound/soc/sof/intel/hda.h | 1 + 3 files changed, 19 insertions(+), 4 deletions(-) -- 2.37.0

2 3

Add profile for Behringer UMC202HD
by GitHub pull_request - opened 12 Jul '22

12 Jul '22

alsa-project/alsa-ucm-conf pull request #188 was opened from Eiseiche: Edited #128 for UMC202HD support (thx @brndd for initial work). [alsa-info.txt](https://github.com/alsa-project/alsa-ucm-conf/files/9094061/… [spa-acp-tool.txt](https://github.com/alsa-project/alsa-ucm-conf/files/90940… Request URL : https://github.com/alsa-project/alsa-ucm-conf/pull/188 Patch URL : https://github.com/alsa-project/alsa-ucm-conf/pull/188.patch Repository URL: https://github.com/alsa-project/alsa-ucm-conf

1 0

Re: Sound not working after commit bbf7d3b1c4f40eb02dd1dffb500ba00b0bff0303 on Amlogic A311D device
by Alex Natalsson 12 Jul '22

12 Jul '22

> Wow. So there's already a problem with a config and we made it worse... > > Can you try with this hack attached, just to see what causes the kernel > oops? Thanks! Hello friends) I modified your hack a little. And I don't understanding what is fe and be, but I see what problem apearing due to be_substream points to 0. The "if (!fe_substream->pcm->nonatomic && be_substream->pcm->nonatomic) {" at result gives the error... * I am sorry what delete recipients in the previous send((

4 7

NetBSD/OpenBSD/FreeBSD build support
by GitHub pull_request - opened 12 Jul '22

12 Jul '22

alsa-project/alsa-lib pull request #250 was opened from jg1uaa: to build alsa-lib on *BSD, small fix is required. (reference: FreeBSD/NetBSD ports diff) There is no problem to compile modified code on Linux(Debian-11), FreeBSD-13.1, NetBSD-current and OpenBSD-7.1. Request URL : https://github.com/alsa-project/alsa-lib/pull/250 Patch URL : https://github.com/alsa-project/alsa-lib/pull/250.patch Repository URL: https://github.com/alsa-project/alsa-lib

1 0

[PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found
by Alexandru Elisei 12 Jul '22

12 Jul '22

Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched BCLK to GPIO functions when probing the i2s bus interface, but missed adding a check for when devm_pinctrl_get() returns an error. This can lead to the following NULL pointer dereference on a rockpro64-v2 if there are no "pinctrl" properties in the i2s device tree node: [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 [ 0.663061] Mem abort info: [ 0.663317] ESR = 0x0000000096000004 [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits [ 0.664136] SET = 0, FnV = 0 [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA [ 0.664409] EA = 0, S1PTW = 0 [ 0.664415] FSC = 0x04: level 0 translation fault [ 0.664421] Data abort info: [ 0.666050] ISV = 0, ISS = 0x00000004 [ 0.666399] CM = 0, WnR = 0 [ 0.666671] [0000000000000005] user address but active_mm is swapper [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 0.667742] Modules linked in: [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c [ 0.670527] sp : ffff80000a17bb30 [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed [ 0.677304] Call trace: [ 0.677531] pinctrl_lookup_state+0x20/0xc0 [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c [ 0.678297] platform_probe+0x68/0xc0 [ 0.678638] really_probe.part.0+0x9c/0x2ac [ 0.679027] __driver_probe_device+0x98/0x144 [ 0.679429] driver_probe_device+0xac/0x140 [ 0.679814] __driver_attach+0xf8/0x184 [ 0.680169] bus_for_each_dev+0x70/0xd0 [ 0.680524] driver_attach+0x24/0x30 [ 0.680856] bus_add_driver+0x150/0x200 [ 0.681210] driver_register+0x78/0x130 [ 0.681560] __platform_driver_register+0x28/0x34 [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 [ 0.682407] do_one_initcall+0x50/0x1c0 [ 0.682760] kernel_init_freeable+0x204/0x288 [ 0.683160] kernel_init+0x28/0x13c [ 0.683482] ret_from_fork+0x10/0x20 [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) [ 0.684365] ---[ end trace 0000000000000000 ]--- [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 0.685500] SMP: stopping secondary CPUs [ 0.685995] Kernel Offset: disabled [ 0.686310] CPU features: 0x800,00105811,00001086 [ 0.686736] Memory Limit: none [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- Check that i2s->pinctrl is valid before attempting to search for the bclk_on and bclk_off pinctrl states. Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") Signed-off-by: Alexandru Elisei <alexandru.elisei(a)arm.com> --- Full log at [1], config at [2] (pastebins expire after 6 months). I'm not familiar with this part of the kernel, I did my best to come up with an explanation and a fix for the panic. Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 that is causing the panic (which is, "rockchip,rk3399-i2s" "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe I'm reading the docs wrong, or maybe the board devicetree also needs fixing. [1] https://pastebin.com/vuRVDsKk [2] https://pastebin.com/3yDMF7YE sound/soc/rockchip/rockchip_i2s.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 99a128a666fb..c9fedf6eb2e6 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) i2s->bclk_ratio = 64; i2s->pinctrl = devm_pinctrl_get(&pdev->dev); - if (IS_ERR(i2s->pinctrl)) + if (IS_ERR(i2s->pinctrl)) { dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); + ret = PTR_ERR(i2s->pinctrl); + goto err_clk; + } i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, "bclk_on"); -- 2.37.0

3 11