[PATCH] ALSA: aoa: Handle empty codec list in i2sbus_pcm_prepare()

[PATCH] ALSA: aoa: Handle empty codec list in i2sbus_pcm_prepare()

[Thorsten Blum]

Replace two list_for_each_entry() loops with list_first_entry_or_null()
in i2sbus_pcm_prepare().

Handle an empty codec list explicitly by returning -ENODEV, which avoids
using uninitialized 'bi.sysclock_factor' in the 32-bit code path.

Fixes: f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 sound/aoa/soundbus/i2sbus/pcm.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/aoa/soundbus/i2sbus/pcm.c b/sound/aoa/soundbus/i2sbus/pcm.c
index aff99003d833..65653601662d 100644
--- a/sound/aoa/soundbus/i2sbus/pcm.c
+++ b/sound/aoa/soundbus/i2sbus/pcm.c
@@ -314,7 +314,7 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev, int in)
 	int i, periodsize, nperiods;
 	dma_addr_t offset;
 	struct bus_info bi;
-	struct codec_info_item *cii;
+	struct codec_info_item *cii = NULL;
 	int sfr = 0;		/* serial format register */
 	int dws = 0;		/* data word sizes reg */
 	int input_16bit;
@@ -390,13 +390,11 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev, int in)
 	case SNDRV_PCM_FORMAT_U16_BE:
 		/* FIXME: if we add different bus factors we need to
 		 * do more here!! */
-		bi.bus_factor = 0;
-		list_for_each_entry(cii, &i2sdev->sound.codec_list, list) {
-			bi.bus_factor = cii->codec->bus_factor;
-			break;
-		}
-		if (!bi.bus_factor)
+		cii = list_first_entry_or_null(&i2sdev->sound.codec_list,
+					       struct codec_info_item, list);
+		if (!cii)
 			return -ENODEV;
+		bi.bus_factor = cii->codec->bus_factor;
 		input_16bit = 1;
 		break;
 	case SNDRV_PCM_FORMAT_S32_BE:
@@ -410,10 +408,12 @@ static int i2sbus_pcm_prepare(struct i2sbus_dev *i2sdev, int in)
 		return -EINVAL;
 	}
 	/* we assume all sysclocks are the same! */
-	list_for_each_entry(cii, &i2sdev->sound.codec_list, list) {
-		bi.sysclock_factor = cii->codec->sysclock_factor;
-		break;
-	}
+	if (!cii)
+		cii = list_first_entry_or_null(&i2sdev->sound.codec_list,
+					       struct codec_info_item, list);
+	if (!cii)
+		return -ENODEV;
+	bi.sysclock_factor = cii->codec->sysclock_factor;
 
 	if (clock_and_divisors(bi.sysclock_factor,
 			       bi.bus_factor,

Re: [PATCH] ALSA: aoa: Handle empty codec list in i2sbus_pcm_prepare()

[Takashi Iwai]

On Mon, 09 Mar 2026 12:41:59 +0100,
Thorsten Blum wrote:
> 
> Replace two list_for_each_entry() loops with list_first_entry_or_null()
> in i2sbus_pcm_prepare().

Hmm, I guess both can be simply list_first_entry(), as the codec list
in this code path is guaranteed to be non-empty (it's called after
i2sbus_pcm_open() which has the check of the valid codecs).

> Handle an empty codec list explicitly by returning -ENODEV, which avoids
> using uninitialized 'bi.sysclock_factor' in the 32-bit code path.

Which 32bit code path are you referring to...?


thanks,

Takashi

Re: [PATCH] ALSA: aoa: Handle empty codec list in i2sbus_pcm_prepare()

[Thorsten Blum]

On 9. Mar 2026, at 12:59, Takashi Iwai wrote:
> On Mon, 09 Mar 2026 12:41:59 +0100, Thorsten Blum wrote:
>> Replace two list_for_each_entry() loops with list_first_entry_or_null()
>> in i2sbus_pcm_prepare().
> 
> Hmm, I guess both can be simply list_first_entry(), as the codec list
> in this code path is guaranteed to be non-empty (it's called after
> i2sbus_pcm_open() which has the check of the valid codecs).

That guarantee only holds for open/prepare, not for i2sbus_resume() via
i2sbus_pcm_prepare_both(). It's probably uncommon in practice, but
i2sbus_pcm_prepare() should still handle it safely.

>> Handle an empty codec list explicitly by returning -ENODEV, which avoids
>> using uninitialized 'bi.sysclock_factor' in the 32-bit code path.
> 
> Which 32bit code path are you referring to...?

The SNDRV_PCM_FORMAT_S32_BE/SNDRV_PCM_FORMAT_U32_BE branch.

Thanks,
Thorsten

Re: [PATCH] ALSA: aoa: Handle empty codec list in i2sbus_pcm_prepare()

[Takashi Iwai]

On Mon, 09 Mar 2026 13:55:02 +0100,
Thorsten Blum wrote:
> 
> On 9. Mar 2026, at 12:59, Takashi Iwai wrote:
> > On Mon, 09 Mar 2026 12:41:59 +0100, Thorsten Blum wrote:
> >> Replace two list_for_each_entry() loops with list_first_entry_or_null()
> >> in i2sbus_pcm_prepare().
> > 
> > Hmm, I guess both can be simply list_first_entry(), as the codec list
> > in this code path is guaranteed to be non-empty (it's called after
> > i2sbus_pcm_open() which has the check of the valid codecs).
> 
> That guarantee only holds for open/prepare, not for i2sbus_resume() via
> i2sbus_pcm_prepare_both(). It's probably uncommon in practice, but
> i2sbus_pcm_prepare() should still handle it safely.

Then we should fix i2sbus_resume() instead.  It can simply bail out
when the codec list empty.  Ditto for i2bus_suspend().

> >> Handle an empty codec list explicitly by returning -ENODEV, which avoids
> >> using uninitialized 'bi.sysclock_factor' in the 32-bit code path.
> > 
> > Which 32bit code path are you referring to...?
> 
> The SNDRV_PCM_FORMAT_S32_BE/SNDRV_PCM_FORMAT_U32_BE branch.

The description is confusing :)  It's about 32bit PCM *format*, not
about 32bit code path.


thanks,

Takashi