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

[Yannick FERTRE]

Hello Heinrich,

Sorry for the delay.

This match is not superfluous. On the STM32F746 board, a bitmap larger 
than the panel resolution is embedded.

Without this patch, the board does not boot.

I propose to send an additional patch that checks the coordinates.

Best regards

-- 
Yannick Fertré Microcontrollers and Digital ICs Group | 
Microcontrolleurs Division


On 10/24/19 10:43 PM, Heinrich Schuchardt wrote:
> On 10/24/19 4:05 PM, Patrice CHOTARD wrote:
>> 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.
>
> I still do not understand why this patch is needed.
>
> Could you, please, try to analyze why the board does not boot.
>
> What is wrong with the existing code for clipping?
> Or is the non-booting just coincidence but the bug is somewhere else?
>
> What are the values of the parameters passed to video_bmp_display()?
> Which bitmap file are you using?
> What is the size of the display?
>
> Best regards
>
> Heinrich
>
>>
>> Heinrich, could this patch be merged, waiting for a clean patch from 
>> Yannick ?
>>
>> Regards
>>
>> Patrice
>>