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  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]
Date:   Tue, 18 Aug 2020 09:41:51 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        jiri@...lanox.com, petrm@...lanox.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core

On Mon, Aug 17, 2020 at 10:38:24AM -0500, Bjorn Helgaas wrote:
> You've likely seen this already, but Coverity found this problem:
> 
>   *** CID 1466147:  Control flow issues  (DEADCODE)
>   /drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c: 380 in mlxsw_sp_policers_init()
>   374     	}
>   375     
>   376     	return 0;
>   377     
>   378     err_family_register:
>   379     	for (i--; i >= 0; i--) {
>   >>>     CID 1466147:  Control flow issues  (DEADCODE)
>   >>>     Execution cannot reach this statement: "struct mlxsw_sp_policer_fam...".
>   380     		struct mlxsw_sp_policer_family *family;
>   381     
>   382     		family = mlxsw_sp->policer_core->family_arr[i];
>   383     		mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
>   384     	}
>   385     err_init:
> 
> I think the problem is that MLXSW_SP_POLICER_TYPE_MAX is 0 because
> 
> > +enum mlxsw_sp_policer_type {
> > +	MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> > +
> > +	__MLXSW_SP_POLICER_TYPE_MAX,
> > +	MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
> > +};
> 
> so we can only execute the family_register loop once, with i == 0,
> and if we get to err_family_register via the error exit:
> 
> > +	for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
> > +		err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
> > +		if (err)
> > +			goto err_family_register;
> 
> i will be 0, so i-- sets i to -1, so we don't enter the
> family_unregister loop body since -1 is not >= 0.

Thanks for the report, but isn't the code doing the right thing here? I
mean, it's dead code now, but as soon as we add another family it will
be executed. It seems error prone to remove it only to please Coverity
and then add it back when it's actually needed.

> 
> This code is now upstream as 8d3fbae70d8d ("mlxsw: spectrum_policer:
> Add policer core").
> 
> Bjorn

Powered by blists - more mailing lists