From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Fri, 25 Oct 2019 11:31:10 +0200 Subject: [U-Boot] [PATCH v5 01/15] video: bmp: check resolutions of panel/bitmap In-Reply-To: <366fbfdd-586e-5775-1040-47f55168d889@st.com> References: <1570454955-21298-1-git-send-email-yannick.fertre@st.com> <1570454955-21298-2-git-send-email-yannick.fertre@st.com> <2d9bb18b-5775-00b6-2fdd-1b38dce1abcc@gmx.de> <2ef8752e-a6fd-fec3-ad94-63dcc48fdc13@st.com> <721d8c58-9ba4-6b86-133e-f0a1d4c3de94@gmx.de> <366fbfdd-586e-5775-1040-47f55168d889@st.com> Message-ID: <5096de31-bd03-4808-cc38-b31d310aa235@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On 10/25/19 9:43 AM, Patrice CHOTARD wrote: > Hi Heinrich > > 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é >>>>> --- >>>>>    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? > > > To sum-up, on all STM32 boards, the same BMP splashcreen is used. > > In the particular case of STM32F746-Disco board, the panel size is smaller then the BMP size. > > So, some size check must be done to avoid overflow when writing inside the framebuffer. That is why we have lines to clip images: if ((x + width) > pwidth) width = pwidth - x; if ((y + height) > priv->ysize) height = priv->ysize - y; Why is this not working in you case? Best regards Heinrich > > If needed, Yannick, which is multimedia expert, can give you more precise details. > > Thanks > > Patrice > > >> >> Best regards >> >> Heinrich >> >>> >>> Heinrich, could this patch be merged, waiting for a clean patch from Yannick ? >>> >>> Regards >>> >>> Patrice >>> >> >