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

Patrice CHOTARD patrice.chotard at st.com
Thu Oct 24 14:05:41 UTC 2019


Hi Heinrich, all

On 10/7/19 7:34 PM, Heinrich Schuchardt wrote:
> On 10/7/19 3:29 PM, 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 | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
>> index 193f37d..4af1fb4 100644
>> --- a/drivers/video/video_bmp.c
>> +++ b/drivers/video/video_bmp.c
>> @@ -249,6 +249,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 exceeds panel size */
>> +    if ((pwidth < width) || (priv->ysize < height)) {
>> +        printf("Error: BMP size %d x %d is bigger than panel size %d x %d\n",
>> +               (int)width, (int)height, priv->xsize, priv->ysize);
>> +        return -EINVAL;
>> +    }
>> +
>>       if (align) {
>>           video_splash_align_axis(&x, priv->xsize, width);
>>           video_splash_align_axis(&y, priv->ysize, height);
>
> This is followed by:
>
>         if ((x + width) > pwidth)
>                 width = pwidth - x;
>         if ((y + height) > priv->ysize)
>                 height = priv->ysize - y;
>
> These lines will clip a bitmap that given the left upper corner x, y
> does not fit onto the screen.
>
> So isn't this patch superfluous?
>
> What is missing are checks for x and y.
>
> For testing I have used qemu_x86 and added
>
>     #define CONFIG_BMP_24BPP y
>
> to the top of drivers/video/video_bmp.c and loaded a 24bit bitmap.
>
> Clipping works as expected. But passing a value of x exceeding the
> screen width lead to a crash.
>
> What I really dislike in the existing coding is that CONFIG_BMP_*BPP is
> defined in includes instead of using Kconfig but that is a different story.
>
> Best regards
>
> Heinrich

For information all this series patches have been applied on u-boot/master excepts  this one.

Unfortunately, without this patch, stm32f746-discovery board doesn't boot anymore.

Heinrich, could this patch be merged, waiting for a clean patch from Yannick ?

Regards

Patrice


More information about the U-Boot mailing list