Re: [Qemu-devel] [PATCH] ppc405_uc: Fix buffer overflow

[Andreas Färber]

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 <address@hidden>
> ---
> 
> 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 <address@hidden>

We could alternatively leave besr[2] and access it with hardcoded 0..1.

Adding qemu-stable to the mix so it can be backported to stable-1.2
after the release.

Andreas

> @@ -205,8 +206,10 @@ static uint32_t dcr_read_pob (void *opaque, int dcrn)
>          ret = pob->bear;
>          break;
>      case POB0_BESR0:
> +        ret = pob->besr0;
> +        break;
>      case POB0_BESR1:
> -        ret = pob->besr[dcrn - POB0_BESR0];
> +        ret = pob->besr1;
>          break;
>      default:
>          /* Avoid gcc warning */
> @@ -227,9 +230,12 @@ static void dcr_write_pob (void *opaque, int dcrn, 
> uint32_t val)
>          /* Read only */
>          break;
>      case POB0_BESR0:
> +        /* Write-clear */
> +        pob->besr0 &= ~val;
> +        break;
>      case POB0_BESR1:
>          /* Write-clear */
> -        pob->besr[dcrn - POB0_BESR0] &= ~val;
> +        pob->besr1 &= ~val;
>          break;
>      }
>  }
> @@ -241,8 +247,8 @@ static void ppc4xx_pob_reset (void *opaque)
>      pob = opaque;
>      /* No error */
>      pob->bear = 0x00000000;
> -    pob->besr[0] = 0x0000000;
> -    pob->besr[1] = 0x0000000;
> +    pob->besr0 = 0x0000000;
> +    pob->besr1 = 0x0000000;
>  }
>  
>  static void ppc4xx_pob_init(CPUPPCState *env)
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg