diff mbox series

[U-Boot,v4,01/15] video: bmp: check resolutions of panel/bitmap

Message ID 1568368083-11075-2-git-send-email-yannick.fertre@st.com
State Superseded
Delegated to: Anatolij Gustschin
Headers show
Series splash screen on the stm32f769 & stm32mp1 boards | expand

Commit Message

Yannick FERTRE Sept. 13, 2019, 9:47 a.m. UTC
If the size of the bitmap is bigger than the size of
the panel then errors appear when calculating axis alignment
and the copy of bitmap is done outside of framebuffer.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 drivers/video/video_bmp.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Simon Glass Sept. 17, 2019, 5:48 a.m. UTC | #1
On Fri, 13 Sep 2019 at 02:48, Yannick Fertré <yannick.fertre@st.com> wrote:
>
> If the size of the bitmap is bigger than the size of
> the panel then errors appear when calculating axis alignment
> and the copy of bitmap is done outside of framebuffer.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>  drivers/video/video_bmp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>
Heinrich Schuchardt Sept. 17, 2019, 9:12 p.m. UTC | #2
On 9/13/19 11:47 AM, Yannick Fertré wrote:
> If the size of the bitmap is bigger than the size of
> the panel then errors appear when calculating axis alignment
> and the copy of bitmap is done outside of framebuffer.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   drivers/video/video_bmp.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
> index 193f37d..544bd5f 100644
> --- a/drivers/video/video_bmp.c
> +++ b/drivers/video/video_bmp.c
> @@ -54,6 +54,13 @@ static void video_display_rle8_bitmap(struct udevice *dev,
>   	height = get_unaligned_le32(&bmp->header.height);
>   	bmap = (uchar *)bmp + get_unaligned_le32(&bmp->header.data_offset);
>
> +	/* check if picture size exceed panel size */

%s/exceed/exceeds/

> +	if (priv->xsize < width)
> +		width = priv->xsize;

In case of BMP_RLE8_DELTA width is not checked before writing to the
frame buffer. So this modification of width will lead to unexpected effects.

In the 'default' case width is checked and this may lead to decoding errors.

> +
> +	if (priv->ysize < height)
> +		height = priv->ysize;
> +
>   	x = 0;
>   	y = height - 1;
>
> @@ -249,6 +256,13 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
>
>   	padded_width = (width & 0x3 ? (width & ~0x3) + 4 : width);
>
> +	/* check if picture size exceed panel size */
> +	if (pwidth < width)
> +		width = pwidth;
> +
> +	if (priv->ysize < height)
> +		height = priv->ysize;
> +

You forgot to consider the position (x,y) of the picture.

Best regards

Heinrich

>   	if (align) {
>   		video_splash_align_axis(&x, priv->xsize, width);
>   		video_splash_align_axis(&y, priv->ysize, height);
>
Yannick FERTRE Sept. 24, 2019, 1:43 p.m. UTC | #3
Hello Heinrich,

Thank for your review.

You're right my patch does not allow to properly protect the framebuffer.

I will push a new patch which check the bitmap size and exit in case of 
size error.

Yannick Fertré


On 9/17/19 11:12 PM, Heinrich Schuchardt wrote:
> On 9/13/19 11:47 AM, Yannick Fertré wrote:
>> If the size of the bitmap is bigger than the size of
>> the panel then errors appear when calculating axis alignment
>> and the copy of bitmap is done outside of framebuffer.
>>
>> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
>> ---
>>   drivers/video/video_bmp.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
>> index 193f37d..544bd5f 100644
>> --- a/drivers/video/video_bmp.c
>> +++ b/drivers/video/video_bmp.c
>> @@ -54,6 +54,13 @@ static void video_display_rle8_bitmap(struct 
>> udevice *dev,
>>       height = get_unaligned_le32(&bmp->header.height);
>>       bmap = (uchar *)bmp + 
>> get_unaligned_le32(&bmp->header.data_offset);
>>
>> +    /* check if picture size exceed panel size */
>
> %s/exceed/exceeds/
>
>> +    if (priv->xsize < width)
>> +        width = priv->xsize;
>
> In case of BMP_RLE8_DELTA width is not checked before writing to the
> frame buffer. So this modification of width will lead to unexpected 
> effects.
>
> In the 'default' case width is checked and this may lead to decoding 
> errors.
>
>> +
>> +    if (priv->ysize < height)
>> +        height = priv->ysize;
>> +
>>       x = 0;
>>       y = height - 1;
>>
>> @@ -249,6 +256,13 @@ int video_bmp_display(struct udevice *dev, ulong 
>> bmp_image, int x, int y,
>>
>>       padded_width = (width & 0x3 ? (width & ~0x3) + 4 : width);
>>
>> +    /* check if picture size exceed panel size */
>> +    if (pwidth < width)
>> +        width = pwidth;
>> +
>> +    if (priv->ysize < height)
>> +        height = priv->ysize;
>> +
>
> You forgot to consider the position (x,y) of the picture.
>
> Best regards
>
> Heinrich
>
>>       if (align) {
>>           video_splash_align_axis(&x, priv->xsize, width);
>>           video_splash_align_axis(&y, priv->ysize, height);
>>
diff mbox series

Patch

diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
index 193f37d..544bd5f 100644
--- a/drivers/video/video_bmp.c
+++ b/drivers/video/video_bmp.c
@@ -54,6 +54,13 @@  static void video_display_rle8_bitmap(struct udevice *dev,
 	height = get_unaligned_le32(&bmp->header.height);
 	bmap = (uchar *)bmp + get_unaligned_le32(&bmp->header.data_offset);
 
+	/* check if picture size exceed panel size */
+	if (priv->xsize < width)
+		width = priv->xsize;
+
+	if (priv->ysize < height)
+		height = priv->ysize;
+
 	x = 0;
 	y = height - 1;
 
@@ -249,6 +256,13 @@  int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 
 	padded_width = (width & 0x3 ? (width & ~0x3) + 4 : width);
 
+	/* check if picture size exceed panel size */
+	if (pwidth < width)
+		width = pwidth;
+
+	if (priv->ysize < height)
+		height = priv->ysize;
+
 	if (align) {
 		video_splash_align_axis(&x, priv->xsize, width);
 		video_splash_align_axis(&y, priv->ysize, height);