linux-kernel - Re: [PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned

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] [day] [month] [year] [list]

Message-ID: <aXT4hLBy05W8Spy9@osama>
Date: Sat, 24 Jan 2026 17:51:16 +0100
From: Osama Abdelkader <osama.abdelkader@...il.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
Cc: Simona Vetter <simona@...ll.ch>, Helge Deller <deller@....de>,
	Lee Jones <lee@...nel.org>,
	Murad Masimov <m.masimov@...integration.ru>,
	Quanmin Yan <yanquanmin1@...wei.com>,
	Yongzhen Zhang <zhangyongzhen@...inos.cn>,
	linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org,
	syzbot+55e03490a0175b8dd81d@...kaller.appspotmail.com
Subject: Re: [PATCH] fbdev: Fix slab-out-of-bounds read in
 fb_pad_unaligned_buffer

On Mon, Jan 19, 2026 at 08:45:08AM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 18.01.26 um 14:47 schrieb Osama Abdelkader:
> > The function fb_pad_unaligned_buffer() was reading idx+1 bytes per row
> > from the source buffer, but when mod == 0 (font width is a multiple of
> > 8 bits), the source buffer only has idx bytes per row. This caused a
> > slab-out-of-bounds read when accessing src[idx] after the inner loop.
> > 
> > Fix this by only reading the extra byte when mod != 0, ensuring we
> > never read beyond the source buffer boundaries.
> > 
> > This fixes the KASAN slab-out-of-bounds read reported by syzkaller:
> > https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> > 
> > Reported-by: syzbot+55e03490a0175b8dd81d@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@...il.com>
> > ---
> >   drivers/video/fbdev/core/fbmem.c | 18 ++++++++++--------
> >   1 file changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index eff757ebbed1..a0c4932a6758 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -113,15 +113,17 @@ void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> >   			dst[j+1] = tmp;
> >   			src++;
> >   		}
> > -		tmp = dst[idx];
> > -		tmp &= mask;
> > -		tmp |= *src >> shift_low;
> > -		dst[idx] = tmp;
> > -		if (shift_high < mod) {
> > -			tmp = *src << shift_high;
> > -			dst[idx+1] = tmp;
> > +		if (mod) {
> 
> How do we end up here if mod equals 0? When I look at the callers of this
> function, cases with (mod == 0) take an entirely different branch. [1] [2]
> 
> Best regards
> Thomas
> 
> [1] https://elixir.bootlin.com/linux/v6.18.6/source/drivers/video/fbdev/core/bitblit.c#L208
> [2] https://elixir.bootlin.com/linux/v6.18.6/source/drivers/video/fbdev/core/fbcon_ud.c#L199
> 
> > +			tmp = dst[idx];
> > +			tmp &= mask;
> > +			tmp |= *src >> shift_low;
> > +			dst[idx] = tmp;
> > +			if (shift_high < mod) {
> > +				tmp = *src << shift_high;
> > +				dst[idx+1] = tmp;
> > +			}
> > +			src++;
> >   		}
> > -		src++;
> >   		dst += d_pitch;
> >   	}
> >   }
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
> GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
> 
> 

You’re right that callers should only reach this path when mod != 0.
The issue isn’t the mod == 0 case itself, but that the final source byte is read
and consumed even when shift_high >= mod, where no bits are actually used.
I resent a version that only accesses the extra byte when it contributes data.

Best regards,
osama