[PATCH] fbcon: fix buffer overflow in fbcon_set_font

[PATCH] fbcon: fix buffer overflow in fbcon_set_font

[Simon Richter]

Commit 1a194e6c8e1ee745e914b0b7f50fa86c89ed13fe introduced overflow
checking for the font allocation size calculation, but in doing so moved
the addition of the size for font housekeeping data out of the kmalloc
call.

As a result, the calculated size now includes those extra bytes, which
marks the same number of bytes beyond the allocation as valid font data.

The crc32() call and the later memcmp() in fbcon_set_font() already perform
an out-of-bounds read, the latter is flagged on ppc64el:

    memcmp: detected buffer overflow: 4112 byte read of buffer size 4096

when loading Lat15-Fixed16.psf.gz.

Since the addition of the extra size should only go into the kmalloc()
call, calculate this size in a separate variable.

Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
Fixes: 1a194e6c8e1e ("fbcon: fix integer overflow in fbcon_do_set_font")
Cc: stable <stable@vger.kernel.org> #v5.9+
---
 drivers/video/fbdev/core/fbcon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 5fade44931b8..a3fbf42c57d9 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2518,7 +2518,7 @@ static int fbcon_set_font(struct vc_data *vc, const struct console_font *font,
 	unsigned charcount = font->charcount;
 	int w = font->width;
 	int h = font->height;
-	int size;
+	int size, allocsize;
 	int i, csum;
 	u8 *new_data, *data = font->data;
 	int pitch = PITCH(font->width);
@@ -2551,10 +2551,10 @@ static int fbcon_set_font(struct vc_data *vc, const struct console_font *font,
 		return -EINVAL;
 
 	/* Check for overflow in allocation size calculation */
-	if (check_add_overflow(FONT_EXTRA_WORDS * sizeof(int), size, &size))
+	if (check_add_overflow(FONT_EXTRA_WORDS * sizeof(int), size, &allocsize))
 		return -EINVAL;
 
-	new_data = kmalloc(size, GFP_USER);
+	new_data = kmalloc(allocsize, GFP_USER);
 
 	if (!new_data)
 		return -ENOMEM;
-- 
2.47.3

Re: [PATCH] fbcon: fix buffer overflow in fbcon_set_font

[Thomas Zimmermann]

Hi,

thanks for the patch. The fix at [1] has meanwhile been merged into the 
DRM tree.

[1] https://patchwork.freedesktop.org/series/154848/

Best regards
Thomas

Am 23.09.25 um 17:06 schrieb Simon Richter:
> Commit 1a194e6c8e1ee745e914b0b7f50fa86c89ed13fe introduced overflow
> checking for the font allocation size calculation, but in doing so moved
> the addition of the size for font housekeeping data out of the kmalloc
> call.
>
> As a result, the calculated size now includes those extra bytes, which
> marks the same number of bytes beyond the allocation as valid font data.
>
> The crc32() call and the later memcmp() in fbcon_set_font() already perform
> an out-of-bounds read, the latter is flagged on ppc64el:
>
>      memcmp: detected buffer overflow: 4112 byte read of buffer size 4096
>
> when loading Lat15-Fixed16.psf.gz.
>
> Since the addition of the extra size should only go into the kmalloc()
> call, calculate this size in a separate variable.
>
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> Fixes: 1a194e6c8e1e ("fbcon: fix integer overflow in fbcon_do_set_font")
> Cc: stable <stable@vger.kernel.org> #v5.9+
> ---
>   drivers/video/fbdev/core/fbcon.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 5fade44931b8..a3fbf42c57d9 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2518,7 +2518,7 @@ static int fbcon_set_font(struct vc_data *vc, const struct console_font *font,
>   	unsigned charcount = font->charcount;
>   	int w = font->width;
>   	int h = font->height;
> -	int size;
> +	int size, allocsize;
>   	int i, csum;
>   	u8 *new_data, *data = font->data;
>   	int pitch = PITCH(font->width);
> @@ -2551,10 +2551,10 @@ static int fbcon_set_font(struct vc_data *vc, const struct console_font *font,
>   		return -EINVAL;
>   
>   	/* Check for overflow in allocation size calculation */
> -	if (check_add_overflow(FONT_EXTRA_WORDS * sizeof(int), size, &size))
> +	if (check_add_overflow(FONT_EXTRA_WORDS * sizeof(int), size, &allocsize))
>   		return -EINVAL;
>   
> -	new_data = kmalloc(size, GFP_USER);
> +	new_data = kmalloc(allocsize, GFP_USER);
>   
>   	if (!new_data)
>   		return -ENOMEM;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)