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: <8c792a4a-8a1c-4ccb-b517-eb02bc18b11f@gmail.com>
Date: Tue, 5 Nov 2024 16:18:02 +0530
From: Suraj Sonawane <surajsonawane0215@...il.com>
To: Péter Ujfalusi <peter.ujfalusi@...ux.intel.com>
Cc: broonie@...nel.org, 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 04/11/24 16:22, 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.
>>
>> This solution maintains defined behavior for rate->min and rate->max,
>> ensuring they are only assigned when valid be_rate data is available.
>>
>> Signed-off-by: Suraj Sonawane <surajsonawane0215@...il.com>
>> ---
>> V1: Initialize 'be_rate' to 0.
>> V2: Add conditional assignment based on num_input_formats to ensure
>> be_rate is used only when assigned.
>>
>>   sound/soc/sof/ipc4-pcm.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c
>> index 4df2be3d3..d5d7ffc69 100644
>> --- a/sound/soc/sof/ipc4-pcm.c
>> +++ b/sound/soc/sof/ipc4-pcm.c
>> @@ -633,8 +633,11 @@ static int sof_ipc4_pcm_dai_link_fixup_rate(struct snd_sof_dev *sdev,
>>   			return -EINVAL;
>>   		}
>>   
>> -		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.
> 
>> +			rate->min = be_rate;
>> +			rate->max = rate->min;
>> +		}
>>   	}
>>   
>>   	return 0;
> 
Thank you for the clarification.