fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer

[PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer

Posted by Osama Abdelkader 2 weeks, 6 days ago

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.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) {
+			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;
 	}
 }
-- 
2.43.0

Re: [PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer

Posted by Thomas Zimmermann 2 weeks, 5 days ago

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.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)

Re: [PATCH] fbdev: Fix slab-out-of-bounds read in fb_pad_unaligned_buffer

Posted by Osama Abdelkader 2 weeks ago

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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.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