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

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: <a4039ca2-6c76-4431-8e27-caebe1a56deb@gmail.com>
Date: Thu, 31 Oct 2024 12:17:56 +0530
From: Suraj Sonawane <surajsonawane0215@...il.com>
To: Mark Brown <broonie@...nel.org>
Cc: Liam Girdwood <lgirdwood@...il.com>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sound: fix uninit-value in i2s_dma_isr

On 30/10/24 22:44, Mark Brown wrote:
> On Wed, Oct 30, 2024 at 10:38:29PM +0530, Suraj Sonawane wrote:
>> Fix an issue detected by the Smatch tool:
>>
>> sound/soc/bcm/bcm63xx-pcm-whistler.c:264 i2s_dma_isr()
>> error: uninitialized symbol 'val_1'.
>> sound/soc/bcm/bcm63xx-pcm-whistler.c:264 i2s_dma_isr()
>> error: uninitialized symbol 'val_2'.
>>
>> These errors occurred because the variables 'val_1' and 'val_2' are
>> declared but may not be assigned a value before they are used.
>> Specifically, if the loop that assigns values to 'val_1' and 'val_2'
>> does not execute (for example, when 'offlevel' is zero), these
>> variables remain uninitialized, leading to potential undefined
>> behavior.
>>
>> To resolve this issue, initialize 'val_1' and 'val_2' to 0 at the
>> point of declaration. This ensures that 'val_1' and 'val_2' have
>> defined values before they are used in subsequent calculations,
>> preventing any warnings or undefined behavior in cases where the
>> loop does not run.
> 
> This will shut the warning up, but why are these values valid?  Are we
> handling the cases where the loops do not execute properly?

Thank you for the feedback and your time.

The uninitialized warning for val_1 and val_2 arises because, in some 
cases, the offlevel value is zero, and as a result, the loop does not 
execute, leaving these variables potentially undefined. The subsequent 
code calculates prtd->dma_addr_next using val_1 + val_2, so it's 
necessary to have val_1 and val_2 initialized to a known value, even 
when the loop does not run.

Initializing them to zero ensures prtd->dma_addr_next has a defined 
value without triggering undefined behavior. However, if a zero 
initialization could cause unintended behavior in dma_addr_next, I could 
alternatively handle this case by setting dma_addr_next conditionally 
when offlevel is non-zero.

Let me know if there’s a preferred approach, or if you'd suggest a 
different initial value for these variables based on the expected use.