The DRM splash client parses the same BMP headers as efifb. Rather than keep a second copy of the structures, move them to include/video/bmp.h with the magic number.
BMP is little endian whatever the CPU, so the header gives the fields as __le16 and __le32, and efifb now reads them with get_unaligned_le16() and get_unaligned_le32(). Width and height are read signed, as the format has them. No change on the little endian systems EFI runs on. Suggested-by: Thomas Zimmermann <[email protected]> Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Màxim Pedraza Padilla <[email protected]> --- Notes: TODO for the efifb maintainers, not changed here so that this patch only moves the headers: the bound check if ((bitmap_offset + bmp_pitch * bmp_height) > bgrt_image_size) is done in u32 and can wrap with a crafted BGRT image, for instance with a bitmap_offset close to 4 GiB, and so can round_up(3 * bmp_width, 4) and the product with a huge width; image_offset_x + bmp_width against lfb_width is a u32 sum too. efifb_copy_bmp() would then read outside the mapped image. The DRM splash client checks the same headers with check_add_overflow() and check_mul_overflow(). I can send a separate fix if wanted. drivers/video/fbdev/efifb.c | 49 ++++++++++++++----------------------- include/video/bmp.h | 40 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 31 deletions(-) create mode 100644 include/video/bmp.h diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 3411be06d3e6..fd348a72f780 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -16,32 +16,13 @@ #include <linux/platform_device.h> #include <linux/printk.h> #include <linux/sysfb.h> +#include <linux/unaligned.h> +#include <video/bmp.h> #include <video/vga.h> #include <asm/efi.h> #include <drm/drm_connector.h> /* For DRM_MODE_PANEL_ORIENTATION_* */ #include <drm/drm_panel_quirks.h> /* For drm_get_panel_orientation_quirk */ -struct bmp_file_header { - u16 id; - u32 file_size; - u32 reserved; - u32 bitmap_offset; -} __packed; - -struct bmp_dib_header { - u32 dib_header_size; - s32 width; - s32 height; - u16 planes; - u16 bpp; - u32 compression; - u32 bitmap_size; - u32 horz_resolution; - u32 vert_resolution; - u32 colors_used; - u32 colors_important; -} __packed; - static bool use_bgrt = true; static bool request_mem_succeeded = false; static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC; @@ -145,9 +126,10 @@ static bool efifb_bgrt_sanity_check(const struct screen_info *si, u32 bmp_width) static void efifb_show_boot_graphics(struct fb_info *info, const struct screen_info *si) { - u32 bmp_width, bmp_height, bmp_pitch, dst_x, y, src_y; + u32 bmp_width, bmp_height, bmp_pitch, bitmap_offset, dst_x, y, src_y; struct bmp_file_header *file_header; struct bmp_dib_header *dib_header; + s32 width, height; void *bgrt_image = NULL; u8 *dst = info->screen_base; @@ -186,20 +168,25 @@ static void efifb_show_boot_graphics(struct fb_info *info, const struct screen_i goto error; file_header = bgrt_image; - if (file_header->id != 0x4d42 || file_header->reserved != 0) + if (get_unaligned_le16(&file_header->id) != BMP_FILE_MAGIC_ID || + get_unaligned_le32(&file_header->reserved) != 0) goto error; + bitmap_offset = get_unaligned_le32(&file_header->bitmap_offset); dib_header = bgrt_image + sizeof(*file_header); - if (dib_header->dib_header_size != 40 || dib_header->width < 0 || - dib_header->planes != 1 || dib_header->bpp != 24 || - dib_header->compression != 0) + width = get_unaligned_le32(&dib_header->width); + height = get_unaligned_le32(&dib_header->height); + if (get_unaligned_le32(&dib_header->dib_header_size) != 40 || + width < 0 || get_unaligned_le16(&dib_header->planes) != 1 || + get_unaligned_le16(&dib_header->bpp) != 24 || + get_unaligned_le32(&dib_header->compression) != 0) goto error; - bmp_width = dib_header->width; - bmp_height = abs(dib_header->height); + bmp_width = width; + bmp_height = abs(height); bmp_pitch = round_up(3 * bmp_width, 4); - if ((file_header->bitmap_offset + bmp_pitch * bmp_height) > + if ((bitmap_offset + bmp_pitch * bmp_height) > bgrt_image_size) goto error; @@ -222,12 +209,12 @@ static void efifb_show_boot_graphics(struct fb_info *info, const struct screen_i src_y = y - bgrt_tab.image_offset_y; /* Positive header height means upside down row order */ - if (dib_header->height > 0) + if (height > 0) src_y = (bmp_height - 1) - src_y; memset(dst, 0, bgrt_tab.image_offset_x * 4); dst_x = bgrt_tab.image_offset_x; - efifb_copy_bmp(bgrt_image + file_header->bitmap_offset + + efifb_copy_bmp(bgrt_image + bitmap_offset + src_y * bmp_pitch, (u32 *)dst + dst_x, bmp_width, si); dst_x += bmp_width; diff --git a/include/video/bmp.h b/include/video/bmp.h new file mode 100644 index 000000000000..8aa249f2302f --- /dev/null +++ b/include/video/bmp.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _VIDEO_BMP_H +#define _VIDEO_BMP_H + +#include <linux/types.h> + +/* "BM", the first two bytes of a BMP file */ +#define BMP_FILE_MAGIC_ID 0x4d42 + +/* + * The headers at the start of a BMP file. Every field is little endian, + * whatever the CPU, and the headers need not be aligned in memory: read them + * with get_unaligned_le16() and get_unaligned_le32(). + */ +struct bmp_file_header { + __le16 id; + __le32 file_size; + __le32 reserved; + __le32 bitmap_offset; +} __packed; + +/* + * BITMAPINFOHEADER. Width and height are signed: a negative width means + * nothing, and a negative height that the rows go top to bottom. + */ +struct bmp_dib_header { + __le32 dib_header_size; + __le32 width; + __le32 height; + __le16 planes; + __le16 bpp; + __le32 compression; + __le32 bitmap_size; + __le32 horz_resolution; + __le32 vert_resolution; + __le32 colors_used; + __le32 colors_important; +} __packed; + +#endif /* _VIDEO_BMP_H */ -- 2.39.5
