[PATCH] ASoC: soc-core: start rtd->num_components at zero

[PATCH] ASoC: soc-core: start rtd->num_components at zero

[Dennis Gilmore]

Since the conversion to devm_kzalloc_flex(), the allocator sets the
__counted_by() counter num_components to the allocated length
(num_cpus + num_codecs + num_platforms). This only happens when the
compiler supports __builtin_counted_by_ref() (GCC 16 here). With
older compilers the counter stays zero, as before.

snd_soc_rtd_add_component() treats num_components as the number of
components added so far: it increments the counter and stores the
new component at num_components - 1. Starting from the preset
length, every component is written past the end of the allocation.
On a link with one CPU, one codec and one platform, entries 3 to 5
of a 3-entry array are written, and components[0] stays NULL.

for_each_rtd_components() stops at that NULL entry, so
soc_probe_link_components() probes no component, and
component->card stays NULL for every component on the card. The
first DAI .probe callback that uses its component's card then
dereferences NULL. On an HP OmniBook X 14 (MediaTek MT8901
SoundWire with CS42L43 and CS35L56 codecs) binding the card
crashes in cs42l43_dai_probe():

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000038
  Internal error: Oops: 0000000096000004 [#1]  SMP
  pc : snd_soc_card_get_kcontrol+0x18/0x40 [snd_soc_core]
  lr : snd_soc_component_get_kcontrol+0x6c/0xc0 [snd_soc_core]
  Call trace:
   snd_soc_card_get_kcontrol+0x18/0x40 [snd_soc_core] (P)
   snd_soc_component_get_kcontrol+0x6c/0xc0 [snd_soc_core]
   cs42l43_dai_probe+0xb0/0x148 [snd_soc_cs42l43]
   snd_soc_pcm_dai_probe+0x70/0xf8 [snd_soc_core]
   snd_soc_bind_card+0x4d4/0x8c0 [snd_soc_core]
   call_soc_bind_card+0x70/0xf0 [snd_soc_core]
   snd_soc_register_card+0xd8/0x110 [snd_soc_core]
   devm_snd_soc_register_card+0x20/0x38 [snd_soc_core]
   mt8901_card_probe+0x258/0x5c8 [mt8901_acp_sdw_legacy_mach]
   platform_probe+0x88/0xc0

Reset the counter after allocation, so components are added from
index 0 again as they were before the conversion.

The bug was found, and this fix written, with the help of an AI
coding assistant (Claude). Tested on the HP OmniBook X 14: the card
binds and playback and capture work.

Fixes: 4dd10fbbeb9c ("treewide: Convert devm_kmalloc() family to the devm_kmalloc_obj() family")
Signed-off-by: Dennis Gilmore <dennis@ausil.us>
---
An alternative fix would be to keep num_components as the array size
and have snd_soc_rtd_add_component() fill the first NULL slot. The
counter would still need to be set explicitly, because the preset does
not happen on compilers without __builtin_counted_by_ref(). Resetting it
keeps the original design and the tighter __counted_by() bounds.

Kees, the same pattern (a *_flex() allocation followed by code that uses
the counter as a fill index starting from zero) looks to be present in
two places converted by 69050f8d6d07 ("treewide: Replace kmalloc with
kmalloc_obj for non-scalar types"). I have only read the code for these.
I have no hardware for either and have not reproduced them, so please
treat them as possible issues rather than confirmed bugs:

 - drivers/spi/spi-axi-spi-engine.c: spi_engine_optimize_message()
   allocates p with kzalloc_flex(*p, instructions, p_dry.length + 1),
   which presets p->length, and spi_engine_compile_message() then
   appends with p->length++ through spi_engine_program_add_cmd().

 - drivers/net/wireless/intel/iwlwifi/mld/d3.c:
   iwl_mld_process_netdetect_res() allocates netdetect_info with
   kzalloc_flex(..., matches, n_matches), presetting n_matches, and
   iwl_mld_set_netdetect_info() stores at matches[n_matches] and then
   increments it. The per-match kzalloc_flex(..., channels, n_channels)
   has the same issue with match->n_channels.

If they are real, the fix would be the same as here. I'm happy to send
patches. It may also be worth a Coccinelle check for this pattern.

 sound/soc/soc-core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 68b54e509b65..e4d911577c5f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -388,6 +388,13 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 		return NULL;
 	}
 
+	/*
+	 * devm_kzalloc_flex() presets the __counted_by() counter to the
+	 * allocated size, but snd_soc_rtd_add_component() uses
+	 * num_components as the number of components added so far.
+	 */
+	rtd->num_components = 0;
+
 	rtd->dev = dev;
 	INIT_LIST_HEAD(&rtd->list);
 	for_each_pcm_streams(stream) {
-- 
2.56.0