diff mbox series

[U-Boot,1/8] video/console: Fix DM_VIDEO font glyph array indexing

Message ID 20190323013002.27117-2-andre.przywara@arm.com
State Accepted
Delegated to: Anatolij Gustschin
Headers show
Series video/console: Fix various DM_VIDEO console issues | expand

Commit Message

Andre Przywara March 23, 2019, 1:29 a.m. UTC
When the character to be printed on a DM_VIDEO console is from the
"extended ASCII" range (0x80 - 0xff), it will be treated as a negative
number, as it's declared as a signed char. This leads to negative array
indicies into the glyph bitmap array, and random garbled characters.

Cast the character to an unsigned type to make the index always positive
and avoid an out-of-bounds access.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/video/console_normal.c | 3 ++-
 drivers/video/console_rotate.c | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Simon Glass March 30, 2019, 9:18 p.m. UTC | #1
Hi Andre,

On Fri, 22 Mar 2019 at 19:32, Andre Przywara <andre.przywara@arm.com> wrote:
>
> When the character to be printed on a DM_VIDEO console is from the
> "extended ASCII" range (0x80 - 0xff), it will be treated as a negative
> number, as it's declared as a signed char. This leads to negative array
> indicies into the glyph bitmap array, and random garbled characters.
>
> Cast the character to an unsigned type to make the index always positive
> and avoid an out-of-bounds access.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/video/console_normal.c | 3 ++-
>  drivers/video/console_rotate.c | 7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Should use u8 instead of uint8_t.

It might make sense to adjust one of the tests to output such a character.

Regards,
Simon
Anatolij Gustschin April 9, 2019, 9:03 p.m. UTC | #2
On Sat, 23 Mar 2019 01:29:55 +0000
Andre Przywara andre.przywara@arm.com wrote:

> When the character to be printed on a DM_VIDEO console is from the
> "extended ASCII" range (0x80 - 0xff), it will be treated as a negative
> number, as it's declared as a signed char. This leads to negative array
> indicies into the glyph bitmap array, and random garbled characters.
> 
> Cast the character to an unsigned type to make the index always positive
> and avoid an out-of-bounds access.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/video/console_normal.c | 3 ++-
>  drivers/video/console_rotate.c | 7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)

Applied (with s/uint8_t/u8/) to u-boot-video/master, thanks!

--
Anatolij
diff mbox series

Patch

diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index 2cfa510d5f..511589eaff 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -84,7 +84,8 @@  static int console_normal_putc_xy(struct udevice *dev, uint x_frac, uint y,
 		return -EAGAIN;
 
 	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
-		uchar bits = video_fontdata[ch * VIDEO_FONT_HEIGHT + row];
+		unsigned int idx = (uint8_t)ch * VIDEO_FONT_HEIGHT + row;
+		uchar bits = video_fontdata[idx];
 
 		switch (vid_priv->bpix) {
 #ifdef CONFIG_VIDEO_BPP8
diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c
index f076570335..e5ebaa03fa 100644
--- a/drivers/video/console_rotate.c
+++ b/drivers/video/console_rotate.c
@@ -90,7 +90,7 @@  static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
 	int i, col;
 	int mask = 0x80;
 	void *line;
-	uchar *pfont = video_fontdata + ch * VIDEO_FONT_HEIGHT;
+	uchar *pfont = video_fontdata + (uint8_t)ch * VIDEO_FONT_HEIGHT;
 
 	line = vid_priv->fb + (VID_TO_PIXEL(x_frac) + 1) *
 			vid_priv->line_length - (y + 1) * pbytes;
@@ -222,7 +222,8 @@  static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
 			VIDEO_FONT_WIDTH - 1) * VNBYTES(vid_priv->bpix);
 
 	for (row = 0; row < VIDEO_FONT_HEIGHT; row++) {
-		uchar bits = video_fontdata[ch * VIDEO_FONT_HEIGHT + row];
+		unsigned int idx = (uint8_t)ch * VIDEO_FONT_HEIGHT + row;
+		uchar bits = video_fontdata[idx];
 
 		switch (vid_priv->bpix) {
 #ifdef CONFIG_VIDEO_BPP8
@@ -348,7 +349,7 @@  static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
 	void *line = vid_priv->fb +
 		(vid_priv->ysize - VID_TO_PIXEL(x_frac) - 1) *
 		vid_priv->line_length + y * pbytes;
-	uchar *pfont = video_fontdata + ch * VIDEO_FONT_HEIGHT;
+	uchar *pfont = video_fontdata + (uint8_t)ch * VIDEO_FONT_HEIGHT;
 
 	if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
 		return -EAGAIN;