| Message ID | 20250327100126.12585-2-arefev@swemel.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fbdev: atyfb: Fix buffer overflow | expand |
On Thu, 27 Mar 2025, Denis Arefev <arefev@swemel.ru> wrote: > The value LCD_MISC_CNTL is used in the 'aty_st_lcd()' function to > calculate an index for accessing an array element of size 9. > This may cause a buffer overflow. The fix is to fix it, not silently brush it under the carpet. BR, Jani. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Denis Arefev <arefev@swemel.ru> > --- > drivers/video/fbdev/aty/atyfb_base.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c > index 210fd3ac18a4..93eb5eb6042b 100644 > --- a/drivers/video/fbdev/aty/atyfb_base.c > +++ b/drivers/video/fbdev/aty/atyfb_base.c > @@ -149,6 +149,8 @@ static const u32 lt_lcd_regs[] = { > void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) > { > if (M64_HAS(LT_LCD_REGS)) { > + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) > + return; > aty_st_le32(lt_lcd_regs[index], val, par); > } else { > unsigned long temp; > @@ -164,6 +166,8 @@ void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) > u32 aty_ld_lcd(int index, const struct atyfb_par *par) > { > if (M64_HAS(LT_LCD_REGS)) { > + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) > + return 0; > return aty_ld_le32(lt_lcd_regs[index], par); > } else { > unsigned long temp;
On Thu, Mar 27, 2025 at 12:14:26PM +0200, Jani Nikula wrote: > On Thu, 27 Mar 2025, Denis Arefev <arefev@swemel.ru> wrote: > > The value LCD_MISC_CNTL is used in the 'aty_st_lcd()' function to > > calculate an index for accessing an array element of size 9. > > This may cause a buffer overflow. > > The fix is to fix it, not silently brush it under the carpet. There's actually nothing to fix. The backlight code is only used on Rage Mobility which has real indexed LCD registers. Older chips do supposedly have backlight control as well, but implemented differently. I was mildly curious about this stuff, so I I poked at my Rage LT Pro a bit to see if I could get backlight control working on it, but the only things I was able to achieve were either backlight completely off, or blinking horribly. So looks like at least on this machine (Dell Insipiron 7000) the backlight is implemented in a way that can't be controller via the normal registers. The machine does have brightness keys that do work (though the difference between the min and max is barely noticeable) but they don't result in any changes in the relevant registers. > > BR, > Jani. > > > > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: Denis Arefev <arefev@swemel.ru> > > --- > > drivers/video/fbdev/aty/atyfb_base.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c > > index 210fd3ac18a4..93eb5eb6042b 100644 > > --- a/drivers/video/fbdev/aty/atyfb_base.c > > +++ b/drivers/video/fbdev/aty/atyfb_base.c > > @@ -149,6 +149,8 @@ static const u32 lt_lcd_regs[] = { > > void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) > > { > > if (M64_HAS(LT_LCD_REGS)) { > > + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) > > + return; > > aty_st_le32(lt_lcd_regs[index], val, par); > > } else { > > unsigned long temp; > > @@ -164,6 +166,8 @@ void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) > > u32 aty_ld_lcd(int index, const struct atyfb_par *par) > > { > > if (M64_HAS(LT_LCD_REGS)) { > > + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) > > + return 0; > > return aty_ld_le32(lt_lcd_regs[index], par); > > } else { > > unsigned long temp; > > -- > Jani Nikula, Intel
Hi Ville. Hi Jani. Thank you for your answers. One small question. This chip (3D RAGE LT (Mach64 LG)) is very old it is 25 or maybe 30 years old, why is it not removed from the core? Regards Denis.
On 4/1/25 10:40, Denis Arefev wrote: > One small question. > This chip (3D RAGE LT (Mach64 LG)) is very old it is 25 or > maybe 30 years old, why is it not removed from the core? It's old, but still runs in some configurations and people still (although probably not on daily bases) use it. Also don't forget about the various old non-x86 hardware machines which often used ATI cards too, and those machines are still supported by Linux as well. Helge
> It's old, but still runs in some configurations and people > still (although probably not on daily bases) use it. > Also don't forget about the various old non-x86 hardware machines > which often used ATI cards too, and those machines are still > supported by Linux as well. Hi Helge. Thanks for the reply. Ok. Everyone agrees that there is an error (buffer overflow lt_lcd_regs[LCD_MISC_CNTL]). Ok. Everyone agrees that this code is still needed. Then I propose to fix this error. :) Unfortunately, I can't do everything by the rules, I didn't save the chip datasheet. (I didn't think I would ever need it again.). Regards Denis.
diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c index 210fd3ac18a4..93eb5eb6042b 100644 --- a/drivers/video/fbdev/aty/atyfb_base.c +++ b/drivers/video/fbdev/aty/atyfb_base.c @@ -149,6 +149,8 @@ static const u32 lt_lcd_regs[] = { void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) { if (M64_HAS(LT_LCD_REGS)) { + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) + return; aty_st_le32(lt_lcd_regs[index], val, par); } else { unsigned long temp; @@ -164,6 +166,8 @@ void aty_st_lcd(int index, u32 val, const struct atyfb_par *par) u32 aty_ld_lcd(int index, const struct atyfb_par *par) { if (M64_HAS(LT_LCD_REGS)) { + if ((u32)index >= ARRAY_SIZE(lt_lcd_regs)) + return 0; return aty_ld_le32(lt_lcd_regs[index], par); } else { unsigned long temp;
The value LCD_MISC_CNTL is used in the 'aty_st_lcd()' function to calculate an index for accessing an array element of size 9. This may cause a buffer overflow. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Denis Arefev <arefev@swemel.ru> --- drivers/video/fbdev/aty/atyfb_base.c | 4 ++++ 1 file changed, 4 insertions(+)