linux-kernel - Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove rq_lease

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: <d9060e6f-f1d5-4322-80fa-3794770c0153@app.fastmail.com>
Date: Tue, 13 Jan 2026 16:09:56 -0500
From: "Chuck Lever" <cel@...nel.org>
To: "Jeff Layton" <jlayton@...nel.org>,
 "Chuck Lever" <chuck.lever@...cle.com>, NeilBrown <neil@...wn.name>,
 "Olga Kornievskaia" <okorniev@...hat.com>, "Dai Ngo" <Dai.Ngo@...cle.com>,
 "Tom Talpey" <tom@...pey.com>, "Trond Myklebust" <trondmy@...nel.org>,
 "Anna Schumaker" <anna@...nel.org>
Cc: "Benjamin Coddington" <bcodding@...merspace.com>,
 linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] nfsd/sunrpc: add svc_rqst->rq_private pointer and remove
 rq_lease_breaker



On Tue, Jan 13, 2026, at 3:12 PM, Jeff Layton wrote:
> On Tue, 2026-01-13 at 14:41 -0500, Chuck Lever wrote:
>> 
>> On Tue, Jan 13, 2026, at 2:31 PM, Jeff Layton wrote:
>> > On Tue, 2026-01-13 at 14:21 -0500, Chuck Lever wrote:
>> > > 
>> > > On Tue, Jan 13, 2026, at 1:37 PM, Jeff Layton wrote:
>> > > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> > > > index eee8c3f4a251a3fae6e41679de0ec34c76caf198..8ce366c9e49220e8baf475c2e5f3424fedc1cec1 100644
>> > > > --- a/fs/nfsd/nfssvc.c
>> > > > +++ b/fs/nfsd/nfssvc.c
>> > > > @@ -900,6 +900,7 @@ nfsd(void *vrqstp)
>> > > > struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
>> > > > struct net *net = perm_sock->xpt_net;
>> > > > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>> > > > + struct nfsd_thread_local_info ntli = { };
>> > > > bool have_mutex = false;
>> > > >  
>> > > > /* At this point, the thread shares current->fs
>> > > > @@ -914,6 +915,8 @@ nfsd(void *vrqstp)
>> > > >  
>> > > > set_freezable();
>> > > >  
>> > > > + rqstp->rq_private = &ntli;
>> > > > +
>> > > > /*
>> > > > * The main request loop
>> > > > */
>> > > 
>> > > Thanks for tackling this one. Nits below...
>> > > 
>> > > This assumes sizeof(structure nfsd_thread_local_info) will always
>> > > be small enough that it is reasonable to keep on the stack. I
>> > > can't say that would be a good bet in the long run.
>> > > 
>> > > And we don't need the perfect reliability of not doing a dynamic
>> > > allocation here. If kmalloc(sizeof(struct nfsd_thread_local_info))
>> > > fails, the thread exits immediately, no harm.
>> > > 
>> > 
>> > Not sure how much space Ben will need (if any).
>> > 
>> > We certainly could have nfsd allocate this separately. I didn't see the
>> > point for something that is only a few bytes though.
>> 
>> If we are designing for today, another approach would be to set up
>> a BUILD_WARN_ON or other type of static build failure if this
>> structure grows larger than, say 256 bytes -- then add dynamic
>> allocation at that point.
>> 
>
> I don't see us growing this to huge proportions. I think that's the
> sort of thing we just have to watch out for as maintainers.
>
> If you really insist on a BUILD_WARN_ON tripwire, I'll add one, but it
> seems like overkill to me.

I think a tripwire documents the assumption made here in a single
line... no different than a lockdep_assert.

More critically, though, maintainers tend to have a very short memory.
Not a trustworthy lot, really.

static_assert(sizeof(struct nfsd_thread_local_info) < 256);


-- 
Chuck Lever