On Tue, Mar 05, 2019 at 04:10:20PM +0000, Peter Maydell wrote:
> On Tue, 26 Feb 2019 at 04:53, David Gibson <da...@gibson.dropbear.id.au>
> wrote:
> >
> > From: Greg Kurz <gr...@kaod.org>
>
>
> Hi -- Coverity points out a possible overflow here (CID 1399145):
>
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 00eb3b643c..b92deee771 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3333,14 +3333,26 @@ static void spapr_nmi(NMIState *n, int cpu_index,
> > Error **errp)
> > }
> > }
> >
> > +int spapr_lmb_dt_populate(sPAPRDRConnector *drc, sPAPRMachineState *spapr,
> > + void *fdt, int *fdt_start_offset, Error **errp)
> > +{
> > + uint64_t addr;
> > + uint32_t node;
> > +
> > + addr = spapr_drc_index(drc) * SPAPR_MEMORY_BLOCK_SIZE;
>
> This multiplication is done as a 32x32, which might overflow and
> be truncated before the result is put into the 64-bit result.
> Casting one side or the other to uint64_t would fix this.
I've applied the following fix to my tree and will include it in the
next pull request:
From 07d93b239203f4fb655e42f6a8a194a4f9eb40a2 Mon Sep 17 00:00:00 2001
From: David Gibson <da...@gibson.dropbear.id.au>
Date: Wed, 6 Mar 2019 14:15:26 +1100
Subject: [PATCH] spapr: Force SPAPR_MEMORY_BLOCK_SIZE to be a hwaddr (64-bit)
SPAPR_MEMORY_BLOCK_SIZE is logically a difference in memory addresses, and
hence of type hwaddr which is 64-bit. Previously it wasn't marked as such
which means that it could be treated as 32-bit. That will work in some
circumstances but if multiplied by another 32-bit value it could lead to
a 32-bit overflow and an incorrect result.
One specific instance of this in spapr_lmb_dt_populate() was spotted by
Coverity (CID 1399145).
Reported-by: Peter Maydell <peter.mayd...@linaro.org>
Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
---
include/hw/ppc/spapr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ff1bd60615..1311ebe28e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -792,7 +792,7 @@ int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t
legacy_offset);
#define TYPE_SPAPR_RNG "spapr-rng"
-#define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
+#define SPAPR_MEMORY_BLOCK_SIZE ((hwaddr)1 << 28) /* 256MB */
/*
* This defines the maximum number of DIMM slots we can have for sPAPR
--
2.20.1
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
