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

[Yannick FERTRE]

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 at 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);
>>