All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Stefan Weil <sw@weilnetz.de>
Cc: qemu-trivial@nongnu.org, qemu-stable@nongnu.org,
	Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] ppc405_uc: Fix buffer overflow
Date: Fri, 31 Aug 2012 23:36:13 +0200	[thread overview]
Message-ID: <50412E4D.9010204@suse.de> (raw)
In-Reply-To: <1346444481-31727-1-git-send-email-sw@weilnetz.de>

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 <sw@weilnetz.de>
> ---
> 
> 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 <afaerber@suse.de>

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


WARNING: multiple messages have this Message-ID (diff)
From: "Andreas Färber" <afaerber@suse.de>
To: Stefan Weil <sw@weilnetz.de>
Cc: qemu-trivial@nongnu.org, qemu-stable@nongnu.org,
	Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] ppc405_uc: Fix buffer overflow
Date: Fri, 31 Aug 2012 23:36:13 +0200	[thread overview]
Message-ID: <50412E4D.9010204@suse.de> (raw)
In-Reply-To: <1346444481-31727-1-git-send-email-sw@weilnetz.de>

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 <sw@weilnetz.de>
> ---
> 
> 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 <afaerber@suse.de>

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

  reply	other threads:[~2012-08-31 21:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-31 20:21 [Qemu-trivial] [PATCH] ppc405_uc: Fix buffer overflow Stefan Weil
2012-08-31 20:21 ` [Qemu-devel] " Stefan Weil
2012-08-31 21:36 ` Andreas Färber [this message]
2012-08-31 21:36   ` Andreas Färber
2012-09-01  5:45   ` [Qemu-trivial] " Markus Armbruster
2012-09-01  5:45     ` Markus Armbruster
2012-09-01  6:23     ` [Qemu-trivial] " Alexander Graf
2012-09-01  6:23       ` Alexander Graf
2012-09-01  6:49       ` [Qemu-trivial] " Stefan Weil
2012-09-01  6:49         ` Stefan Weil
2012-09-20  9:33 ` [Qemu-trivial] " Alexander Graf
2012-09-20  9:33   ` [Qemu-devel] " Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50412E4D.9010204@suse.de \
    --to=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.