linux-kernel - Re: Compile Error fs/nfsd/nfs4state.o - clamp() low limit slotsize greater than high limit total_avail/scale

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: <176251424056.634289.13464296772500147856@noble.neil.brown.name>
Date: Fri, 07 Nov 2025 22:17:20 +1100
From: NeilBrown <neilb@...mail.net>
To: "David Laight" <david.laight.linux@...il.com>
Cc: "Chuck Lever" <cel@...nel.org>,
 "stable@...r.kernel.org" <stable@...r.kernel.org>,
 "Andrew Morton" <akpm@...ux-foundation.org>,
 "David Laight" <David.Laight@...LAB.COM>,
 "Linux NFS Mailing List" <linux-nfs@...r.kernel.org>,
 "Linux List Kernel Mailing" <linux-kernel@...r.kernel.org>,
 speedcracker@...mail.com
Subject: Re: Compile Error fs/nfsd/nfs4state.o - clamp() low limit slotsize
 greater than high limit total_avail/scale_factor

On Fri, 07 Nov 2025, David Laight wrote:
> On Thu, 6 Nov 2025 09:33:28 -0500
> Chuck Lever <cel@...nel.org> wrote:
> 
> > FYI
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=220745
> 
> Ugg - that code is horrid.
> It seems to have got deleted since, but it is:
> 
> 	u32 slotsize = slot_bytes(ca);
> 	u32 num = ca->maxreqs;
> 	unsigned long avail, total_avail;
> 	unsigned int scale_factor;
> 
> 	spin_lock(&nfsd_drc_lock);
> 	if (nfsd_drc_max_mem > nfsd_drc_mem_used)
> 		total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used;
> 	else
> 		/* We have handed out more space than we chose in
> 		 * set_max_drc() to allow.  That isn't really a
> 		 * problem as long as that doesn't make us think we
> 		 * have lots more due to integer overflow.
> 		 */
> 		total_avail = 0;
> 	avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION, total_avail);
> 	/*
> 	 * Never use more than a fraction of the remaining memory,
> 	 * unless it's the only way to give this client a slot.
> 	 * The chosen fraction is either 1/8 or 1/number of threads,
> 	 * whichever is smaller.  This ensures there are adequate
> 	 * slots to support multiple clients per thread.
> 	 * Give the client one slot even if that would require
> 	 * over-allocation--it is better than failure.
> 	 */
> 	scale_factor = max_t(unsigned int, 8, nn->nfsd_serv->sv_nrthreads);
> 
> 	avail = clamp_t(unsigned long, avail, slotsize,
> 			total_avail/scale_factor);
> 	num = min_t(int, num, avail / slotsize);
> 	num = max_t(int, num, 1);
> 
> Lets rework it a bit...
> 	if (nfsd_drc_max_mem > nfsd_drc_mem_used) {
> 		total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used;
> 		avail = min(NFSD_MAX_MEM_PER_SESSION, total_avail);
> 		avail = clamp(avail, n + sizeof(xxx), total_avail/8)
> 	} else {
> 		total_avail = 0;
> 		avail = 0;
> 		avail = clamp(0, n + sizeof(xxx), 0);
> 	}
> 
> Neither of those clamp() are sane at all - should be clamp(val, lo, hi)
> with 'lo <= hi' otherwise the result is dependant on the order of the
> comparisons.
> The compiler sees the second one and rightly bleats.

In fact only gcc-9 bleats.

gcc-7 gcc-10 gcc-13 gcc-15
all seem to think it is fine.

NeilBrown