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
| ||
|
Date: Thu, 14 Jul 2022 16:30:49 +0200 From: Christian Lamparter <chunkeey@...il.com> To: Rustam Subkhankulov <subkhankulov@...ras.ru> Cc: Kalle Valo <kvalo@...nel.org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, linux-wireless@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Alexey Khoroshilov <khoroshilov@...ras.ru>, ldv-project@...uxtesting.org Subject: Re: [PATCH v2] p54: add missing parentheses in p54_flush() On 14/07/2022 15:48, Rustam Subkhankulov wrote: > The assignment of the value to the variable total in the loop > condition must be enclosed in additional parentheses, since otherwise, > in accordance with the precedence of the operators, the conjunction > will be performed first, and only then the assignment. > > Due to this error, a warning later in the function after the loop may > not occur in the situation when it should. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Rustam Subkhankulov <subkhankulov@...ras.ru> > Fixes: 0d4171e2153b ("p54: implement flush callback") For reference: This is the "warning" the commit message talks about: | WARN(total, "tx flush timeout, unresponsive firmware"); | } // this is right at the end of the p54_flush() function from what I can tell, the difference between: | while ((total = p54_flush_count(priv) && i--)) { and | while ((total = p54_flush_count(priv)) && i--) { boils down to what that "total" ends up being after the while() has run through. In the original code it can either be 0 (for everything is ok) or 1 (still pending - this is bad / firmware is on the fritz). In the patched version "total" will be 0 or the value of p54_flush_count(priv). I think both the current and the patched version behave the same way and produce the same output. However I think (since the variable is named "total") the returned value of p54_flush_count() is indeed more precise here. Acked-by: Christian Lamparter <chunkeey@...il.com> > --- > drivers/net/wireless/intersil/p54/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c > index a3ca6620dc0c..8fa3ec71603e 100644 > --- a/drivers/net/wireless/intersil/p54/main.c > +++ b/drivers/net/wireless/intersil/p54/main.c > @@ -682,7 +682,7 @@ static void p54_flush(struct ieee80211_hw *dev, struct ieee80211_vif *vif, > * queues have already been stopped and no new frames can sneak > * up from behind. > */ > - while ((total = p54_flush_count(priv) && i--)) { > + while ((total = p54_flush_count(priv)) && i--) { > /* waste time */ > msleep(20); > }
Powered by blists - more mailing lists