netdev - RE: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc839f1e4e3241bf8c0f3eb81f6c5b3f@AcuMS.aculab.com>
Date: Thu, 29 Aug 2024 16:46:40 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'David Howells' <dhowells@...hat.com>, Hongbo Li <lihongbo22@...wei.com>
CC: "johannes@...solutions.net" <johannes@...solutions.net>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "allison.henderson@...cle.com"
	<allison.henderson@...cle.com>, "dsahern@...nel.org" <dsahern@...nel.org>,
	"pshelar@....org" <pshelar@....org>, "linux-wireless@...r.kernel.org"
	<linux-wireless@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "rds-devel@....oracle.com"
	<rds-devel@....oracle.com>, "dccp@...r.kernel.org" <dccp@...r.kernel.org>,
	"dev@...nvswitch.org" <dev@...nvswitch.org>, "linux-afs@...ts.infradead.org"
	<linux-afs@...ts.infradead.org>
Subject: RE: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

From: David Howells <dhowells@...hat.com>
> Sent: 28 August 2024 09:18
> 
> Hongbo Li <lihongbo22@...wei.com> wrote:
> 
> > I see reason is u8, so may I use min_t(u8, sp->ack.reason,
> > RXRPC_ACK__INVALID)?
> 
> No, please don't use min_t(<unsigned type>, ...) if umin() will do.  IIRC,
> some arches can't do byte-level arithmetic.

Not to mention all the places where the wrong type has been used and
significant bits masked off before the comparison.

Is there even a problem with min() here?
It should be fine unless sp->ack.reason is a signed type.
In which case things are probably horribly wrong if it is negative.

Looking at the code I'm not even sure that min() is right.
It really ought to be used for counters/sizes.
This is a bit like the (broken) suggestion of replacing:
	return rval < 0 ? rval : 0;
with
	return min(rval, 0);

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)