[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXkUnxhRTB4Kw6ph@osama>
Date: Tue, 27 Jan 2026 20:40:15 +0100
From: Osama Abdelkader <osama.abdelkader@...il.com>
To: Helge Deller <deller@....de>
Cc: Simona Vetter <simona@...ll.ch>,
Thomas Zimmermann <tzimmermann@...e.de>, Lee Jones <lee@...nel.org>,
"Daniel Thompson (RISCstar)" <danielt@...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 v2] fbdev: avoid out-of-bounds read in
fb_pad_unaligned_buffer()
On Tue, Jan 27, 2026 at 06:57:32PM +0100, Helge Deller wrote:
> On 1/24/26 17:46, Osama Abdelkader wrote:
> > fb_pad_unaligned_buffer() unconditionally reads and advances the source
> > pointer for the final byte of each row, even when no bits from that byte
> > are actually consumed.
> >
> > When shift_high >= mod, the remaining bits do not cross a byte boundary,
> > but the code still accesses the next source byte. This can lead to
> > out-of-bounds reads under malformed geometry, as reported by syzbot.
> >
> > Fix this by only accessing and consuming the final source byte when it
> > contributes bits (shift_high < mod).
> >
> > 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>
> > ---
> > v2: address the real issue (shift_high >= mod) condition.
> > ---
> > drivers/video/fbdev/core/fbmem.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index eff757ebbed1..d125c3db37a1 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -100,7 +100,7 @@ EXPORT_SYMBOL(fb_pad_aligned_buffer);
> > void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> > u32 shift_high, u32 shift_low, u32 mod)
> > {
> > - u8 mask = (u8) (0xfff << shift_high), tmp;
> > + u8 mask = (u8) (0xff << shift_high), tmp;
>
> This part is correct, but shouldn't be part of this patch.
I just sent a seperate patch for that, and going to remove it in next version of this one.
>
>
> > int i, j;
> > for (i = height; i--; ) {
> > @@ -113,15 +113,18 @@ 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;
> > +
> > + /* Only consume another source byte if it contributes bits */
> > if (shift_high < mod) {
> > + tmp = dst[idx];
> > + tmp &= mask;
> > + tmp |= *src >> shift_low;
> > + dst[idx] = tmp;
> > tmp = *src << shift_high;
> > dst[idx+1] = tmp;
> > + src++;
> > }
> > - src++;
>
> Above you moved the src pointer inside the if(), so every line
> processed may miss a ptr increment. This means the source would need to
> be different too, but it hasn't changed, as it's still used from
> bit_putcs_unaligned() which prints a char from the character fonts.
>
> So, I believe this part at least is wrong.
> Did you test it?
>
I couldn't find syzbot's ReproC, so I did minimal one, I will re-test it and write you.
> Helge
BR,
Osama
Powered by blists - more mailing lists