When performing clock calculations, the ppc405_uc code
has several places where it multiplies together two
32-bit variables and assigns the result to a 64-bit
variable. This doesn't quite do what is intended because
C will compute a 32-bit multiply result. Add casts to
ensure we don't truncate the result.
(Spotted by Coverity, CID 1005504, 1005505.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/ppc/ppc405_uc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index d6d3fc2..d5df94a 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1881,7 +1881,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc)
D1 = (((cpc->pllmr >> 20) - 1) & 0xF) + 1; /* FBDV */
D2 = 8 - ((cpc->pllmr >> 16) & 0x7); /* FWDVA */
M = D0 * D1 * D2;
- VCO_out = cpc->sysclk * M;
+ VCO_out = (uint64_t)cpc->sysclk * M;
if (VCO_out < 400000000 || VCO_out > 800000000) {
/* PLL cannot lock */
cpc->pllmr &= ~0x80000000;
@@ -1892,7 +1892,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc)
/* Bypass PLL */
bypass_pll:
M = D0;
- PLL_out = cpc->sysclk * M;
+ PLL_out = (uint64_t)cpc->sysclk * M;
}
CPU_clk = PLL_out;
if (cpc->cr1 & 0x00800000)
@@ -2242,7 +2242,7 @@ static void ppc405ep_compute_clocks (ppc405ep_cpc_t *cpc)
#ifdef DEBUG_CLOCKS_LL
printf("FWDA %01" PRIx32 " %d\n", (cpc->pllmr[1] >> 16) & 0x7, D);
#endif
- VCO_out = cpc->sysclk * M * D;
+ VCO_out = (uint64_t)cpc->sysclk * M * D;
if (VCO_out < 500000000UL || VCO_out > 1000000000UL) {
/* Error - unlock the PLL */
printf("VCO out of range %" PRIu64 "\n", VCO_out);
--
2.7.4
On 02/21/2017 10:33 AM, Peter Maydell wrote: > When performing clock calculations, the ppc405_uc code > has several places where it multiplies together two > 32-bit variables and assigns the result to a 64-bit > variable. This doesn't quite do what is intended because > C will compute a 32-bit multiply result. Add casts to > ensure we don't truncate the result. > > (Spotted by Coverity, CID 1005504, 1005505.) well hidden... > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/ppc/ppc405_uc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c > index d6d3fc2..d5df94a 100644 > --- a/hw/ppc/ppc405_uc.c > +++ b/hw/ppc/ppc405_uc.c > @@ -1881,7 +1881,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc) > D1 = (((cpc->pllmr >> 20) - 1) & 0xF) + 1; /* FBDV */ > D2 = 8 - ((cpc->pllmr >> 16) & 0x7); /* FWDVA */ > M = D0 * D1 * D2; > - VCO_out = cpc->sysclk * M; > + VCO_out = (uint64_t)cpc->sysclk * M; > if (VCO_out < 400000000 || VCO_out > 800000000) { > /* PLL cannot lock */ > cpc->pllmr &= ~0x80000000; > @@ -1892,7 +1892,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc) > /* Bypass PLL */ > bypass_pll: > M = D0; > - PLL_out = cpc->sysclk * M; > + PLL_out = (uint64_t)cpc->sysclk * M; > } > CPU_clk = PLL_out; > if (cpc->cr1 & 0x00800000) > @@ -2242,7 +2242,7 @@ static void ppc405ep_compute_clocks (ppc405ep_cpc_t *cpc) > #ifdef DEBUG_CLOCKS_LL > printf("FWDA %01" PRIx32 " %d\n", (cpc->pllmr[1] >> 16) & 0x7, D); > #endif > - VCO_out = cpc->sysclk * M * D; > + VCO_out = (uint64_t)cpc->sysclk * M * D; > if (VCO_out < 500000000UL || VCO_out > 1000000000UL) { > /* Error - unlock the PLL */ > printf("VCO out of range %" PRIu64 "\n", VCO_out); >
On Tue, Feb 21, 2017 at 01:33:41PM +0000, Peter Maydell wrote: > When performing clock calculations, the ppc405_uc code > has several places where it multiplies together two > 32-bit variables and assigns the result to a 64-bit > variable. This doesn't quite do what is intended because > C will compute a 32-bit multiply result. Add casts to > ensure we don't truncate the result. > > (Spotted by Coverity, CID 1005504, 1005505.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Ouch. Applied to ppc-for-2.9. > --- > hw/ppc/ppc405_uc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c > index d6d3fc2..d5df94a 100644 > --- a/hw/ppc/ppc405_uc.c > +++ b/hw/ppc/ppc405_uc.c > @@ -1881,7 +1881,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc) > D1 = (((cpc->pllmr >> 20) - 1) & 0xF) + 1; /* FBDV */ > D2 = 8 - ((cpc->pllmr >> 16) & 0x7); /* FWDVA */ > M = D0 * D1 * D2; > - VCO_out = cpc->sysclk * M; > + VCO_out = (uint64_t)cpc->sysclk * M; > if (VCO_out < 400000000 || VCO_out > 800000000) { > /* PLL cannot lock */ > cpc->pllmr &= ~0x80000000; > @@ -1892,7 +1892,7 @@ static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc) > /* Bypass PLL */ > bypass_pll: > M = D0; > - PLL_out = cpc->sysclk * M; > + PLL_out = (uint64_t)cpc->sysclk * M; > } > CPU_clk = PLL_out; > if (cpc->cr1 & 0x00800000) > @@ -2242,7 +2242,7 @@ static void ppc405ep_compute_clocks (ppc405ep_cpc_t *cpc) > #ifdef DEBUG_CLOCKS_LL > printf("FWDA %01" PRIx32 " %d\n", (cpc->pllmr[1] >> 16) & 0x7, D); > #endif > - VCO_out = cpc->sysclk * M * D; > + VCO_out = (uint64_t)cpc->sysclk * M * D; > if (VCO_out < 500000000UL || VCO_out > 1000000000UL) { > /* Error - unlock the PLL */ > printf("VCO out of range %" PRIu64 "\n", VCO_out); -- 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
© 2016 - 2024 Red Hat, Inc.