On 31.08.2012, at 22:45, Markus Armbruster <[email protected]> wrote:
> Andreas Färber <[email protected]> writes: > >> Am 31.08.2012 22:21, schrieb Stefan Weil: >>> Report from smatch: >>> >>> ppc405_uc.c:209 dcr_read_pob(12) error: buffer overflow 'pob->besr' 2 <= 2 >>> ppc405_uc.c:232 dcr_write_pob(12) error: buffer overflow 'pob->besr' 2 <= 2 >>> >>> The old code reads and writes besr[POB0_BESR1 - POB0_BESR0] or besr[2] >>> which is one too much. >>> >>> Signed-off-by: Stefan Weil <[email protected]> >>> --- >>> >>> As this code was wrong for more than 5 years, there is no urgent need to >>> fix it now for QEMU 1.2. >>> >>> Regards, >>> >>> Stefan Weil >>> >>> hw/ppc405_uc.c | 16 +++++++++++----- >>> 1 file changed, 11 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c >>> index 89e5013..b52ab2f 100644 >>> --- a/hw/ppc405_uc.c >>> +++ b/hw/ppc405_uc.c >>> @@ -191,7 +191,8 @@ enum { >>> typedef struct ppc4xx_pob_t ppc4xx_pob_t; >>> struct ppc4xx_pob_t { >>> uint32_t bear; >>> - uint32_t besr[2]; >>> + uint32_t besr0; >>> + uint32_t besr1; >>> }; >>> >>> static uint32_t dcr_read_pob (void *opaque, int dcrn) >> >> Reviewed-by: Andreas Färber <[email protected]> >> >> We could alternatively leave besr[2] and access it with hardcoded 0..1. > > Minimally invasive fix would be besr[dcrn != POB0_BESR0]. > > [...] I don't think the change is important enough for these stylistic questions :). I'll just apply it once I'm back to a real internet connection. Alex
