Re: [PATCH] hw/net/rocker: Avoid undefined shifts with more than 31 ports

[Jason Wang]

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;
> >
>