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: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
Signed-off-by: Osama Abdelkader <[email protected]>
---
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
