netdev - RE: [patch] qlcnic: fix a timeout loop

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: <BLUPR11MB0114ED16EBE57CC01FD9797393EE0@BLUPR11MB0114.namprd11.prod.outlook.com>
Date:	Tue, 15 Dec 2015 13:46:08 +0000
From:	Manish Chopra <manish.chopra@...gic.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>,
	Dept-GE Linux NIC Dev <Dept-GELinuxNICDev@...gic.com>,
	Rajesh Borundia <rajesh.borundia@...gic.com>
CC:	netdev <netdev@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: RE: [patch] qlcnic: fix a timeout loop

> -----Original Message-----
> From: dept_hsg_linux_nic_dev-bounces@...listserver.qlogic.com
> [mailto:dept_hsg_linux_nic_dev-bounces@...listserver.qlogic.com] On Behalf
> Of Dan Carpenter
> Sent: Tuesday, December 15, 2015 3:46 PM
> To: Dept-GE Linux NIC Dev <Dept-GELinuxNICDev@...gic.com>; Rajesh
> Borundia <rajesh.borundia@...gic.com>
> Cc: netdev <netdev@...r.kernel.org>; kernel-janitors@...r.kernel.org
> Subject: [patch] qlcnic: fix a timeout loop
> 
> The problem here is that at the end of the loop we test for if
> idc->vnic_wait_limit is zero, but since idc->vnic_wait_limit-- is a
> post-op, it actually ends up set to (u8)-1.  I have fixed this by changing it to a
> pre-op.  I had to change the starting value from
> "QLCNIC_DEV_NPAR_OPER_TIMEO" (30) to "QLCNIC_DEV_NPAR_OPER_TIMEO
> + 1" so that we still loop the same number of times as before.
> 
> Fixes: 486a5bc77a4a ('qlcnic: Add support for 83xx suspend and resume.')
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> 
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c
> index be7d7a6..9919245 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c
> @@ -234,7 +234,7 @@ int qlcnic_83xx_config_vnic_opmode(struct
> qlcnic_adapter *adapter)
>  	}
> 
>  	ahw->idc.vnic_state = QLCNIC_DEV_NPAR_NON_OPER;
> -	ahw->idc.vnic_wait_limit = QLCNIC_DEV_NPAR_OPER_TIMEO;
> +	ahw->idc.vnic_wait_limit = QLCNIC_DEV_NPAR_OPER_TIMEO + 1;
> 
>  	return 0;
>  }
> @@ -246,7 +246,7 @@ int qlcnic_83xx_check_vnic_state(struct qlcnic_adapter
> *adapter)
>  	u32 state;
> 
>  	state = QLCRDX(ahw, QLC_83XX_VNIC_STATE);
> -	while (state != QLCNIC_DEV_NPAR_OPER && idc->vnic_wait_limit--) {
> +	while (state != QLCNIC_DEV_NPAR_OPER && --idc->vnic_wait_limit) {
>  		msleep(1000);
>  		state = QLCRDX(ahw, QLC_83XX_VNIC_STATE);
>  	}

Hi Dan,
It looks bit odd incrementing 1 in QLCNIC_DEV_NPAR_OPER_TIMEO. Can't we just post increment inside the loop ?

ahw->idc.vnic_wait_limit = QLCNIC_DEV_NPAR_OPER_TIMEO;
while (state != QLCNIC_DEV_NPAR_OPER && idc->vnic_wait_limit) {
	idc->vnic_wait_limit--;
	-----;
	-----;
}

Thanks,
Manish 
 

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