You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
83 lines
3.7 KiB
83 lines
3.7 KiB
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
|
From: Zhang Boyang <zhangboyang.id@gmail.com> |
|
Date: Tue, 6 Sep 2022 03:03:21 +0800 |
|
Subject: [PATCH] fbutil: Fix integer overflow |
|
|
|
Expressions like u64 = u32 * u32 are unsafe because their products are |
|
truncated to u32 even if left hand side is u64. This patch fixes all |
|
problems like that one in fbutil. |
|
|
|
To get right result not only left hand side have to be u64 but it's also |
|
necessary to cast at least one of the operands of all leaf operators of |
|
right hand side to u64, e.g. u64 = u32 * u32 + u32 * u32 should be |
|
u64 = (u64)u32 * u32 + (u64)u32 * u32. |
|
|
|
For 1-bit bitmaps grub_uint64_t have to be used. It's safe because any |
|
combination of values in (grub_uint64_t)u32 * u32 + u32 expression will |
|
not overflow grub_uint64_t. |
|
|
|
Other expressions like ptr + u32 * u32 + u32 * u32 are also vulnerable. |
|
They should be ptr + (grub_addr_t)u32 * u32 + (grub_addr_t)u32 * u32. |
|
|
|
This patch also adds a comment to grub_video_fb_get_video_ptr() which |
|
says it's arguments must be valid and no sanity check is performed |
|
(like its siblings in grub-core/video/fb/fbutil.c). |
|
|
|
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com> |
|
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> |
|
(cherry picked from commit 50a11a81bc842c58962244a2dc86bbd31a426e12) |
|
--- |
|
grub-core/video/fb/fbutil.c | 4 ++-- |
|
include/grub/fbutil.h | 13 +++++++++---- |
|
2 files changed, 11 insertions(+), 6 deletions(-) |
|
|
|
diff --git a/grub-core/video/fb/fbutil.c b/grub-core/video/fb/fbutil.c |
|
index b98bb51fe8..25ef39f47d 100644 |
|
--- a/grub-core/video/fb/fbutil.c |
|
+++ b/grub-core/video/fb/fbutil.c |
|
@@ -67,7 +67,7 @@ get_pixel (struct grub_video_fbblit_info *source, |
|
case 1: |
|
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) |
|
{ |
|
- int bit_index = y * source->mode_info->width + x; |
|
+ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; |
|
grub_uint8_t *ptr = source->data + bit_index / 8; |
|
int bit_pos = 7 - bit_index % 8; |
|
color = (*ptr >> bit_pos) & 0x01; |
|
@@ -138,7 +138,7 @@ set_pixel (struct grub_video_fbblit_info *source, |
|
case 1: |
|
if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) |
|
{ |
|
- int bit_index = y * source->mode_info->width + x; |
|
+ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; |
|
grub_uint8_t *ptr = source->data + bit_index / 8; |
|
int bit_pos = 7 - bit_index % 8; |
|
*ptr = (*ptr & ~(1 << bit_pos)) | ((color & 0x01) << bit_pos); |
|
diff --git a/include/grub/fbutil.h b/include/grub/fbutil.h |
|
index 4205eb917f..78a1ab3b45 100644 |
|
--- a/include/grub/fbutil.h |
|
+++ b/include/grub/fbutil.h |
|
@@ -31,14 +31,19 @@ struct grub_video_fbblit_info |
|
grub_uint8_t *data; |
|
}; |
|
|
|
-/* Don't use for 1-bit bitmaps, addressing needs to be done at the bit level |
|
- and it doesn't make sense, in general, to ask for a pointer |
|
- to a particular pixel's data. */ |
|
+/* |
|
+ * Don't use for 1-bit bitmaps, addressing needs to be done at the bit level |
|
+ * and it doesn't make sense, in general, to ask for a pointer |
|
+ * to a particular pixel's data. |
|
+ * |
|
+ * This function assumes that bounds checking has been done in previous phase |
|
+ * and they are opted out in here. |
|
+ */ |
|
static inline void * |
|
grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source, |
|
unsigned int x, unsigned int y) |
|
{ |
|
- return source->data + y * source->mode_info->pitch + x * source->mode_info->bytes_per_pixel; |
|
+ return source->data + (grub_addr_t) y * source->mode_info->pitch + (grub_addr_t) x * source->mode_info->bytes_per_pixel; |
|
} |
|
|
|
/* Advance pointer by VAL bytes. If there is no unaligned access available,
|
|
|