[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/display/bcm2835_fb.c: Initialize all fields of struct
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] hw/display/bcm2835_fb.c: Initialize all fields of struct |
Date: |
Mon, 29 Jun 2020 09:14:00 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/28/20 9:54 PM, Peter Maydell wrote:
> In bcm2835_fb_mbox_push(), Coverity complains (CID 1429989) that we
> pass a pointer to a local struct to another function without
> initializing all its fields. This is a real bug:
> bcm2835_fb_reconfigure() copies the whole of our new BCM2385FBConfig
> struct into s->config, so any fields we don't initialize will corrupt
> the state of the device.
>
> Copy the two fields which we don't want to update (pixo and alpha)
> from the existing config so we don't accidentally change them.
>
> Fixes: cfb7ba983857e40e88
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Not sure why this wasn't a visible bug -- alpha isn't used,
> but if pixo changes from zero to non-zero we flip from
> RGB to BGR...
> ---
> hw/display/bcm2835_fb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c
> index c6263808a27..7c0e5eef2d5 100644
> --- a/hw/display/bcm2835_fb.c
> +++ b/hw/display/bcm2835_fb.c
> @@ -282,6 +282,10 @@ static void bcm2835_fb_mbox_push(BCM2835FBState *s,
> uint32_t value)
> newconf.base = s->vcram_base | (value & 0xc0000000);
> newconf.base += BCM2835_FB_OFFSET;
>
> + /* Copy fields which we don't want to change from the existing config */
> + newconf.pixo = s->config.pixo;
> + newconf.alpha = s->config.alpha;
> +
> bcm2835_fb_validate_config(&newconf);
>
> pitch = bcm2835_fb_get_pitch(&newconf);
>