linux-kernel - Re: [PATCH net-next v11 4/5] net: rnpgbe: Add basic mbx

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

Message-ID: <vfarjsi3uf55kb5uj25stnjriemyvra7gomxmtik3jowsp24n5@k44vc2gdmyaf>
Date: Wed, 10 Sep 2025 10:21:58 +0200
From: Jörg Sommer <joerg@...so.de>
To: Yibo Dong <dong100@...se.com>
Cc: Jakub Kicinski <kuba@...nel.org>, "Anwar, Md Danish" <a0501179@...com>, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, 
	horms@...nel.org, corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com, 
	mpe@...erman.id.au, danishanwar@...com, lee@...ger.us, gongfan1@...wei.com, 
	lorenzo@...nel.org, geert+renesas@...der.be, Parthiban.Veerasooran@...rochip.com, 
	lukas.bulwahn@...hat.com, alexanderduyck@...com, richardcochran@...il.com, kees@...nel.org, 
	gustavoars@...nel.org, rdunlap@...radead.org, vadim.fedorenko@...ux.dev, 
	netdev@...r.kernel.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH net-next v11 4/5] net: rnpgbe: Add basic mbx_fw support

Yibo Dong schrieb am Mi 10. Sep, 14:08 (+0800):
> On Tue, Sep 09, 2025 at 01:58:22PM -0700, Jakub Kicinski wrote:
> > On Tue, 9 Sep 2025 19:59:11 +0530 Anwar, Md Danish wrote:
> > > > +int mucse_mbx_sync_fw(struct mucse_hw *hw)
> > > > +{
> > > > +	int try_cnt = 3;
> > > > +	int err;
> > > > +
> > > > +	do {
> > > > +		err = mucse_mbx_get_info(hw);
> > > > +		if (err == -ETIMEDOUT)
> > > > +			continue;
> > > > +		break;
> > > > +	} while (try_cnt--);
> > > > +
> > > > +	return err;
> > > > +}  
> > > 
> > > There's a logical issue in the code. The loop structure attempts to
> > > retry on ETIMEDOUT errors, but the unconditional break statement after
> > > the if-check will always exit the loop after the first attempt,
> > > regardless of the error. The do-while loop will never actually retry
> > > because the break statement is placed outside of the if condition that
> > > checks for timeout errors.
> > 
> 
> What is expected is 'retry on ETIMEDOUT' and 'no retry others'. 
> https://lore.kernel.org/netdev/a066746c-2f12-4e70-b63a-7996392a9132@lunn.ch/
> 
> > The other way around. continue; in a do {} while () look does *not*
> > evaluate the condition. So this can loop forever.
> > 
> 
> Maybe I can update like this ?
> 
> int mucse_mbx_sync_fw(struct mucse_hw *hw)
> {
> 	int try_cnt = 3;
> 	int err;
> 
> 	do {
> 		err = mucse_mbx_get_info(hw);
> 		if (err != -ETIMEDOUT)
> 			break;
> 		/* only retry with ETIMEDOUT, others just return */
> 	} while (try_cnt--);
> 
> 	return err;
> }  

How about something like this?

int mucse_mbx_sync_fw(struct mucse_hw *hw)
{
	for (int try = 3; try; --try) {
		int err = mucse_mbx_get_info(hw);
		if (err != -ETIMEDOUT)
			return err;
	}

	return ETIMEDOUT;
}


My 2cent.

Regards Jörg

-- 
„Es wurden und werden zu viele sprachlose Bücher gedruckt, nach deren
schon flüchtiger Lektüre man all die Bäume um Vergebung bitten möchte,
die für den Schund ihr Leben lassen mussten.“ (Michael Jürgs,
                      Seichtgebiete – Warum wir hemmungslos verblöden)

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)