Re: [PATCH net 1/2] linux/dim: Fix overflow in dim calculation

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jul 23, 2019 at 05:22:43PM +0000, Saeed Mahameed wrote:
> On Tue, 2019-07-23 at 10:22 +0300, Leon Romanovsky wrote:
> > From: Yamin Friedman <yaminf@mellanox.com>
> >
> > While using net_dim, a dim_sample was used without ever initializing
> > the
> > comps value. Added use of DIV_ROUND_DOWN_ULL() to prevent potential
> > overflow, it should not be a problem to save the final result in an
> > int
> > because after the division by epms the value should not be larger
> > than a
> > few thousand.
> >
> > [ 1040.127124] UBSAN: Undefined behaviour in lib/dim/dim.c:78:23
> > [ 1040.130118] signed integer overflow:
> > [ 1040.131643] 134718714 * 100 cannot be represented in type 'int'
> >
> > Fixes: 398c2b05bbee ("linux/dim: Add completions count to
> > dim_sample")
> > Signed-off-by: Yamin Friedman <yaminf@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/net/ethernet/broadcom/bcmsysport.c        | 2 +-
> >  drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 2 +-
> >  drivers/net/ethernet/broadcom/genet/bcmgenet.c    | 2 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 4 ++--
> >  lib/dim/dim.c                                     | 4 ++--
> >  5 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
> > b/drivers/net/ethernet/broadcom/bcmsysport.c
> > index b9c5cea8db16..9483553ce444 100644
> > --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> > +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> > @@ -992,7 +992,7 @@ static int bcm_sysport_poll(struct napi_struct
> > *napi, int budget)
> >  {
> >  	struct bcm_sysport_priv *priv =
> >  		container_of(napi, struct bcm_sysport_priv, napi);
> > -	struct dim_sample dim_sample;
> > +	struct dim_sample dim_sample = {};
>
> net_dim implementation doesn't care about sample->comp_ctr, so this is
> unnecessary for the sake of fixing the rdma overflow issue, but it
> doens't hurt anyone to have this change in this patch.

Yes, this is why we decided to change all drivers and not mlx5 only.

Thanks