Hi Simon,
Thanks! Fixed in v3.
And good catch on the unbounded RLE reads. I'll send that as a separate
follow-up.
Thanks,
Shah
On 28.07.26 13:54, Simon Glass wrote:
Hi shj,
On 2026-07-22T07:10:10, shj <[email protected]> wrote:
video: bmp: bound RLE8 decode writes to the framebuffer
video_display_rle8_bitmap() moves the framebuffer cursor fb up by a
full row plus a scanline on each End-Of-Line escape, with no
lower-bound check. Repeated EOL escapes desynchronise fb from the
scanline index y: after height - 1 escapes y is back in range while fb
has drifted about one framebuffer below priv->fb, and the decoder
writes pixel data before the start of the framebuffer. The DELTA
escape recomputes fb from an unchecked y as well. A crafted RLE8
image displayed from the splash-screen or PXE-menu path can therefore
write out of bounds.
Check the cursor against [priv->fb, priv->fb + fb_size) before each
run and reject the image with -EINVAL if a write would fall outside
it.
Signed-off-by: shj <[email protected]>
drivers/video/video_bmp.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
@@ -337,8 +347,10 @@ int video_bmp_display(struct udevice *dev, ulong
bmp_image, int x, int y,
- video_display_rle8_bitmap(dev, bmp, bpix,
palette, fb,
- x, y, width, height);
+ if (video_display_rle8_bitmap(dev, bmp, bpix,
+ palette, fb, x, y,
+ width, height))
+ return -EINVAL;
This now returns a proper error code, so please propagate it rather
than flattening every non-zero value to -EINVAL:
int ret;
...
ret = video_display_rle8_bitmap(dev, bmp, bpix, palette, fb,
x, y, width, height);
if (ret)
return ret;
With this:
Reviewed-by: Simon Glass <[email protected]>
diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
@@ -176,6 +178,9 @@ static void video_display_rle8_bitmap(struct udevice *dev,
+ if (fb < fb_start ||
+ fb + cnt * bytes_per_pixel >
fb_end)
+ return -EINVAL;
BTW this bounds the writes correctly, but the reads from the RLE
stream (bmap) are still unbounded: the decoder advances bmap and the
encoded-run aggregation reads bmap[2]/bmap[3] with no check against
the end of the loaded image, so a crafted image from the same
splash/PXE path can still drive an out-of-bounds read. That is beyond
the scope of this patch, so a follow-up is fine.
Regards,
Simon
