linux-kernel - Re: [PATCH v2] sound: fix uninit-value in sof_ipc4_pcm_dai_link_fixup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d19b77e2-496b-4633-a69c-576cc79c004a@sirena.org.uk>
Date: Mon, 4 Nov 2024 18:27:28 +0000
From: Mark Brown <broonie@...nel.org>
To: Péter Ujfalusi <peter.ujfalusi@...ux.intel.com>
Cc: Suraj Sonawane <surajsonawane0215@...il.com>, daniel.baluta@....com,
	kai.vehmanen@...ux.intel.com, lgirdwood@...il.com,
	linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org,
	perex@...ex.cz, pierre-louis.bossart@...ux.dev,
	ranjani.sridharan@...ux.intel.com,
	sound-open-firmware@...a-project.org, tiwai@...e.com,
	yung-chuan.liao@...ux.intel.com
Subject: Re: [PATCH v2] sound: fix uninit-value in
 sof_ipc4_pcm_dai_link_fixup_rate

On Mon, Nov 04, 2024 at 12:52:09PM +0200, Péter Ujfalusi wrote:
> On 03/11/2024 13:37, Suraj Sonawane wrote:

> > Fix an issue detected by the Smatch tool:
> > 
> > sound/soc/sof/ipc4-pcm.c: sof_ipc4_pcm_dai_link_fixup_rate()
> > error: uninitialized symbol 'be_rate'.
> > 
> > This issue occurred because the variable 'be_rate' could remain
> > uninitialized if num_input_formats is zero. In such cases, the
> > loop that assigns a value to 'be_rate' would not execute,
> > potentially leading to undefined behavior when rate->min and
> > rate->max are set with an uninitialized 'be_rate'.
> > 
> > To resolve this, an additional check for num_input_formats > 0
> > was added before setting rate->min and rate->max with 'be_rate'.
> > This ensures that 'be_rate' is assigned only when there are valid
> > input formats, preventing any use of uninitialized data.

> > -		rate->min = be_rate;
> > -		rate->max = rate->min;
> > +		/* Set rate only if be_rate was assigned */
> > +		if (num_input_formats > 0) {

> By definition the copier must have at least one input and one output
> format, this check is going to be always true.

Static analysis of the code can't reasonably tell that, we need
something that ensures that it doesn't detect a spuriously uninitialised
variable here.  Possibly a

	if (WARN_ON_ONCE(!num_input_formats))
		return -EINVAL;

or similar?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)