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.mayd...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4...@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); >
