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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190723180436.GT5125@mtr-leonro.mtl.com>
Date:   Tue, 23 Jul 2019 21:04:36 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Yamin Friedman <yaminf@...lanox.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Tal Gilboa <talgi@...lanox.com>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net 1/2] linux/dim: Fix overflow in dim calculation

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@...lanox.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@...lanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@...lanox.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