fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()

[PATCH v2] fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()

Posted by Osama Abdelkader 1 week, 6 days ago

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

Re: [PATCH v2] fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()

Posted by Helge Deller 1 week, 3 days ago

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


>   	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?

Helge

Re: [PATCH v2] fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()

Posted by Osama Abdelkader 1 week, 3 days ago

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