netdev - RE: [PATCH 1/2] bnx2x:Fix error handling in the function bnxc2x_dbcx_set

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>] [day] [month] [year] [list]

Date:	Sun, 19 Jul 2015 06:51:16 +0000
From:	Yuval Mintz <Yuval.Mintz@...gic.com>
To:	Nicholas Krause <xerofoify@...il.com>,
	Ariel Elior <Ariel.Elior@...gic.com>
CC:	netdev <netdev@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] bnx2x:Fix error handling in the function
 bnxc2x_dbcx_set_params

> This fixes the error handling in the function bnxc2x_dbcx_params for both calls
> to bnx2x_dcbnl_update_applist by checking if they failed by returning a error
> code and if so return immediately to this function's caller due to this
> nonrecoverable internal failure.

Hi Nicholas,

Even assuming this is the reasonable thing to do [which I'm not fully convinced],
I don't think this solution is complete - you'd also need to break the iteration in
bnx2x_dcbnl_update_applist() [at the moment said function always returns 'rc'
from the last dcb_app weve tried setting].

Also, why do you send multiple patches for fixing the same issue in different
functions in the same file?

Thanks,
Yuval

> 
> Signed-off-by: Nicholas Krause <xerofoify@...il.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
> index 6e4294e..e3cd0c6 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
> @@ -724,7 +724,8 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32
> state)
>  			 * Delete app tlvs from dcbnl before reading new
>  			 * negotiation results
>  			 */
> -			bnx2x_dcbnl_update_applist(bp, true);
> +			if (bnx2x_dcbnl_update_applist(bp, true))
> +				return;
> 
>  			/* Read remote mib if dcbx is in the FW */
>  			if (bnx2x_dcbx_read_shmem_remote_mib(bp))
> @@ -748,7 +749,8 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32
> state)
>  			/*
>  			 * Add new app tlvs to dcbnl
>  			 */
> -			bnx2x_dcbnl_update_applist(bp, false);
> +			if (bnx2x_dcbnl_update_applist(bp, false))
> +				return;
>  #endif
>  			/*
>  			 * reconfigure the netdevice with the results of the new
> --
> 2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html