| Message ID | 20220729155932.2477385-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/net/rocker: Avoid undefined shifts with more than 31 ports | expand |
Ping? thanks -- PMM On Fri, 29 Jul 2022 at 16:59, Peter Maydell <peter.maydell@linaro.org> wrote: > > In rocker_port_phys_link_status() and rocker_port_phys_enable_read() > we construct a 64-bit value with one bit per front-panel port. > However we accidentally do the shift as 32-bit arithmetic, which > means that if there are more than 31 front-panel ports this is > undefined behaviour. > > Fix the problem by ensuring we use 64-bit arithmetic for the whole > calculation. (We won't ever shift off the 64-bit value because > ROCKER_FP_PORTS_MAX is 62.) > > Resolves: Coverity CID 1487121, 1487160 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/net/rocker/rocker.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c > index 31f2340fb91..d8f3f16fe87 100644 > --- a/hw/net/rocker/rocker.c > +++ b/hw/net/rocker/rocker.c > @@ -1010,7 +1010,7 @@ static uint64_t rocker_port_phys_link_status(Rocker *r) > FpPort *port = r->fp_port[i]; > > if (fp_port_get_link_up(port)) { > - status |= 1 << (i + 1); > + status |= 1ULL << (i + 1); > } > } > return status; > @@ -1025,7 +1025,7 @@ static uint64_t rocker_port_phys_enable_read(Rocker *r) > FpPort *port = r->fp_port[i]; > > if (fp_port_enabled(port)) { > - ret |= 1 << (i + 1); > + ret |= 1ULL << (i + 1); > } > } > return ret;
On 8/4/22 03:45, Peter Maydell wrote: > Ping? > > thanks > -- PMM > > On Fri, 29 Jul 2022 at 16:59, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> In rocker_port_phys_link_status() and rocker_port_phys_enable_read() >> we construct a 64-bit value with one bit per front-panel port. >> However we accidentally do the shift as 32-bit arithmetic, which >> means that if there are more than 31 front-panel ports this is >> undefined behaviour. >> >> Fix the problem by ensuring we use 64-bit arithmetic for the whole >> calculation. (We won't ever shift off the 64-bit value because >> ROCKER_FP_PORTS_MAX is 62.) >> >> Resolves: Coverity CID 1487121, 1487160 >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ >> --- >> hw/net/rocker/rocker.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c >> index 31f2340fb91..d8f3f16fe87 100644 >> --- a/hw/net/rocker/rocker.c >> +++ b/hw/net/rocker/rocker.c >> @@ -1010,7 +1010,7 @@ static uint64_t rocker_port_phys_link_status(Rocker *r) >> FpPort *port = r->fp_port[i]; >> >> if (fp_port_get_link_up(port)) { >> - status |= 1 << (i + 1); >> + status |= 1ULL << (i + 1); >> } >> } >> return status; >> @@ -1025,7 +1025,7 @@ static uint64_t rocker_port_phys_enable_read(Rocker *r) >> FpPort *port = r->fp_port[i]; >> >> if (fp_port_enabled(port)) { >> - ret |= 1 << (i + 1); >> + ret |= 1ULL << (i + 1); >> } >> } >> return ret; >
On Thu, Aug 4, 2022 at 11:27 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/4/22 03:45, Peter Maydell wrote: > > Ping? > > > > thanks > > -- PMM > > > > On Fri, 29 Jul 2022 at 16:59, Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >> In rocker_port_phys_link_status() and rocker_port_phys_enable_read() > >> we construct a 64-bit value with one bit per front-panel port. > >> However we accidentally do the shift as 32-bit arithmetic, which > >> means that if there are more than 31 front-panel ports this is > >> undefined behaviour. > >> > >> Fix the problem by ensuring we use 64-bit arithmetic for the whole > >> calculation. (We won't ever shift off the 64-bit value because > >> ROCKER_FP_PORTS_MAX is 62.) > >> > >> Resolves: Coverity CID 1487121, 1487160 > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Queued. Thanks > > > r~ > > >> --- > >> hw/net/rocker/rocker.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c > >> index 31f2340fb91..d8f3f16fe87 100644 > >> --- a/hw/net/rocker/rocker.c > >> +++ b/hw/net/rocker/rocker.c > >> @@ -1010,7 +1010,7 @@ static uint64_t rocker_port_phys_link_status(Rocker *r) > >> FpPort *port = r->fp_port[i]; > >> > >> if (fp_port_get_link_up(port)) { > >> - status |= 1 << (i + 1); > >> + status |= 1ULL << (i + 1); > >> } > >> } > >> return status; > >> @@ -1025,7 +1025,7 @@ static uint64_t rocker_port_phys_enable_read(Rocker *r) > >> FpPort *port = r->fp_port[i]; > >> > >> if (fp_port_enabled(port)) { > >> - ret |= 1 << (i + 1); > >> + ret |= 1ULL << (i + 1); > >> } > >> } > >> return ret; > > >
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c index 31f2340fb91..d8f3f16fe87 100644 --- a/hw/net/rocker/rocker.c +++ b/hw/net/rocker/rocker.c @@ -1010,7 +1010,7 @@ static uint64_t rocker_port_phys_link_status(Rocker *r) FpPort *port = r->fp_port[i]; if (fp_port_get_link_up(port)) { - status |= 1 << (i + 1); + status |= 1ULL << (i + 1); } } return status; @@ -1025,7 +1025,7 @@ static uint64_t rocker_port_phys_enable_read(Rocker *r) FpPort *port = r->fp_port[i]; if (fp_port_enabled(port)) { - ret |= 1 << (i + 1); + ret |= 1ULL << (i + 1); } } return ret;
In rocker_port_phys_link_status() and rocker_port_phys_enable_read() we construct a 64-bit value with one bit per front-panel port. However we accidentally do the shift as 32-bit arithmetic, which means that if there are more than 31 front-panel ports this is undefined behaviour. Fix the problem by ensuring we use 64-bit arithmetic for the whole calculation. (We won't ever shift off the 64-bit value because ROCKER_FP_PORTS_MAX is 62.) Resolves: Coverity CID 1487121, 1487160 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/rocker/rocker.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)