linux-kernel - Re: [PATCH 1/1] fbdev: atyfb: Fix buffer overflow

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Z-rzIfUMmOq1UZY1@intel.com>
Date: Mon, 31 Mar 2025 22:55:13 +0300
From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
To: Jani Nikula <jani.nikula@...el.com>
Cc: Denis Arefev <arefev@...mel.ru>, Helge Deller <deller@....de>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH 1/1] fbdev: atyfb: Fix buffer overflow

On Thu, Mar 27, 2025 at 12:14:26PM +0200, Jani Nikula wrote:
> On Thu, 27 Mar 2025, Denis Arefev <arefev@...mel.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@...mel.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

-- 
Ville Syrjälä
Intel