[BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()

[BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()

[Li Tuo]

Hello,

Our static analysis tool finds a possible uninitialized-variable access 
in the sis driver in Linux 5.14.0-rc3:

At the beginning of the function SiS_SetCRT2FIFO_300(), the variable 
modeidindex is not initialized.
If the following conditions are false, it remains uninitialized.
5346:    if(!SiS_Pr->CRT1UsesCustomMode)
5438:    if(!SiS_Pr->UseCustomMode)

But it is accessed at:
5466:    colorth = SiS_GetColorDepth(SiS_Pr,CRT2ModeNo,modeidindex) >> 1;

I am not quite sure whether this possible uninitialized-variable access 
is real and how to fix it if it is real.
Any feedback would be appreciated, thanks!

Reported-by: TOTE Robot <oslab@tsinghua.edu.cn>

Best wishes,
Tuo Li

Re: [BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()

[Sam Ravnborg]

Hi Tuo Li,

On Sat, Jul 31, 2021 at 02:28:39PM +0800, Li Tuo wrote:
> Hello,
> 
> Our static analysis tool finds a possible uninitialized-variable access in
> the sis driver in Linux 5.14.0-rc3:
> 
> At the beginning of the function SiS_SetCRT2FIFO_300(), the variable
> modeidindex is not initialized.
> If the following conditions are false, it remains uninitialized.
> 5346:    if(!SiS_Pr->CRT1UsesCustomMode)
> 5438:    if(!SiS_Pr->UseCustomMode)
> 
> But it is accessed at:
> 5466:    colorth = SiS_GetColorDepth(SiS_Pr,CRT2ModeNo,modeidindex) >> 1;
> 
> I am not quite sure whether this possible uninitialized-variable access is
> real and how to fix it if it is real.
> Any feedback would be appreciated, thanks!

First, the report looks correct. There is a path where modeindex may not
be initilized.
But I find it very hard to care for such an ancient driver.
If this was somethign we hit is real life we had heard about it - and
the risk of introducing bugs is higher than the the cance that this
fixes a real life bug.

So my advice, find something more relevant to look at.

	Sam

Re: [BUG] video: fbdev: sis: possible uninitialized-variable access in SiS_SetCRT2FIFO_300()

[Li Tuo]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

Thanks for your feedback! Should I commit a patch to fix this problem?

Best wishes,
Tuo Li


On 2021/8/2 1:59, Sam Ravnborg wrote:
> Hi Tuo Li,
>
> On Sat, Jul 31, 2021 at 02:28:39PM +0800, Li Tuo wrote:
>> Hello,
>>
>> Our static analysis tool finds a possible uninitialized-variable access in
>> the sis driver in Linux 5.14.0-rc3:
>>
>> At the beginning of the function SiS_SetCRT2FIFO_300(), the variable
>> modeidindex is not initialized.
>> If the following conditions are false, it remains uninitialized.
>> 5346:    if(!SiS_Pr->CRT1UsesCustomMode)
>> 5438:    if(!SiS_Pr->UseCustomMode)
>>
>> But it is accessed at:
>> 5466:    colorth = SiS_GetColorDepth(SiS_Pr,CRT2ModeNo,modeidindex) >> 1;
>>
>> I am not quite sure whether this possible uninitialized-variable access is
>> real and how to fix it if it is real.
>> Any feedback would be appreciated, thanks!
> First, the report looks correct. There is a path where modeindex may not
> be initilized.
> But I find it very hard to care for such an ancient driver.
> If this was somethign we hit is real life we had heard about it - and
> the risk of introducing bugs is higher than the the cance that this
> fixes a real life bug.
>
> So my advice, find something more relevant to look at.
>
> 	Sam


[-- Attachment #2: Type: text/html, Size: 1701 bytes --]