Patchwork target-arm: Fix potential buffer overflow

login
register
mail settings
Submitter Stefan Weil
Date Sept. 4, 2012, 5:35 a.m.
Message ID <1346736957-24745-1-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/181494/
State Accepted
Headers show

Comments

Stefan Weil - Sept. 4, 2012, 5:35 a.m.
Report from smatch:

target-arm/helper.c:651 arm946_prbs_read(6) error:
 buffer overflow 'env->cp15.c6_region' 8 <= 8
target-arm/helper.c:661 arm946_prbs_write(6) error:
 buffer overflow 'env->cp15.c6_region' 8 <= 8

c7_region is an array with 8 elements, so the index must be less than 8.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 target-arm/helper.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Peter Maydell - Sept. 4, 2012, 2:19 p.m.
On 4 September 2012 06:35, Stefan Weil <sw@weilnetz.de> wrote:
> Report from smatch:
>
> target-arm/helper.c:651 arm946_prbs_read(6) error:
>  buffer overflow 'env->cp15.c6_region' 8 <= 8
> target-arm/helper.c:661 arm946_prbs_write(6) error:
>  buffer overflow 'env->cp15.c6_region' 8 <= 8
>
> c7_region is an array with 8 elements, so the index must be less than 8.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Nice catch, dumb bug I introduced in the conversion to
CPRegInfo in commit 06d76f319f.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Aurelien Jarno - Sept. 10, 2012, 1:17 p.m.
On Tue, Sep 04, 2012 at 07:35:57AM +0200, Stefan Weil wrote:
> Report from smatch:
> 
> target-arm/helper.c:651 arm946_prbs_read(6) error:
>  buffer overflow 'env->cp15.c6_region' 8 <= 8
> target-arm/helper.c:661 arm946_prbs_write(6) error:
>  buffer overflow 'env->cp15.c6_region' 8 <= 8
> 
> c7_region is an array with 8 elements, so the index must be less than 8.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  target-arm/helper.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index dceaa95..e27df96 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -645,7 +645,7 @@ static int pmsav5_insn_ap_read(CPUARMState *env, const ARMCPRegInfo *ri,
>  static int arm946_prbs_read(CPUARMState *env, const ARMCPRegInfo *ri,
>                              uint64_t *value)
>  {
> -    if (ri->crm > 8) {
> +    if (ri->crm >= 8) {
>          return EXCP_UDEF;
>      }
>      *value = env->cp15.c6_region[ri->crm];
> @@ -655,7 +655,7 @@ static int arm946_prbs_read(CPUARMState *env, const ARMCPRegInfo *ri,
>  static int arm946_prbs_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                               uint64_t value)
>  {
> -    if (ri->crm > 8) {
> +    if (ri->crm >= 8) {
>          return EXCP_UDEF;
>      }
>      env->cp15.c6_region[ri->crm] = value;

Thanks, applied.

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index dceaa95..e27df96 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -645,7 +645,7 @@  static int pmsav5_insn_ap_read(CPUARMState *env, const ARMCPRegInfo *ri,
 static int arm946_prbs_read(CPUARMState *env, const ARMCPRegInfo *ri,
                             uint64_t *value)
 {
-    if (ri->crm > 8) {
+    if (ri->crm >= 8) {
         return EXCP_UDEF;
     }
     *value = env->cp15.c6_region[ri->crm];
@@ -655,7 +655,7 @@  static int arm946_prbs_read(CPUARMState *env, const ARMCPRegInfo *ri,
 static int arm946_prbs_write(CPUARMState *env, const ARMCPRegInfo *ri,
                              uint64_t value)
 {
-    if (ri->crm > 8) {
+    if (ri->crm >= 8) {
         return EXCP_UDEF;
     }
     env->cp15.c6_region[ri->crm] = value;