* [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows
@ 2017-02-21 13:33 Peter Maydell
2017-02-21 22:20 ` Philippe Mathieu-Daudé
2017-02-22 0:57 ` David Gibson
0 siblings, 2 replies; 3+ messages in thread
From: Peter Maydell @ 2017-02-21 13:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Alexander Graf, David Gibson, qemu-ppc
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows
2017-02-21 13:33 [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows Peter Maydell
@ 2017-02-21 22:20 ` Philippe Mathieu-Daudé
2017-02-22 0:57 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-21 22:20 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: David Gibson, qemu-ppc, Alexander Graf, patches
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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows
2017-02-21 13:33 [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows Peter Maydell
2017-02-21 22:20 ` Philippe Mathieu-Daudé
@ 2017-02-22 0:57 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2017-02-22 0:57 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches, Alexander Graf, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-22 1:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-21 13:33 [Qemu-devel] [PATCH] hw/ppc/ppc405_uc.c: Avoid integer overflows Peter Maydell
2017-02-21 22:20 ` Philippe Mathieu-Daudé
2017-02-22 0:57 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).