linux-kernel - Re: [PATCH] nfsd: move init of percpu reply_cache_stats counters back to nfsd_init

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: <7d9515dbe6af110f2c32dce2b994120e4637b295.camel@kernel.org>
Date:   Sun, 18 Jun 2023 08:09:50 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Thorsten Leemhuis <regressions@...mhuis.info>,
        Chuck Lever <cel@...nel.org>
Cc:     Chuck Lever <chuck.lever@...cle.com>, Neil Brown <neilb@...e.de>,
        Olga Kornievskaia <kolga@...app.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
        stable@...r.kernel.org, Eirik Fuller <efuller@...hat.com>,
        linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfsd: move init of percpu reply_cache_stats counters
 back to nfsd_init_net

On Sun, 2023-06-18 at 12:40 +0200, Thorsten Leemhuis wrote:
> On 16.06.23 22:54, Jeff Layton wrote:
> > On Fri, 2023-06-16 at 16:27 -0400, Chuck Lever wrote:
> > > Thanks Eirik and Jeff.
> > > 
> > > At this point in the release cycle, I plan to apply this for the
> > > next merge window (6.5).
> > 
> > I think we should take this in sooner. This is a regression and a
> > user-triggerable oops in the right situation. If:
> > 
> > - non-x86_64 arch
> > - /proc/fs/nfsd is mounted in the namespace
> > - nfsd is not started in the namespace
> > - unprivileged user calls "cat /proc/fs/nfsd/reply_cache_stats"
> 
> FWIW, might be worth to simply tell Linus about it and let him decide,
> that's totally fine and even documented in the old and the new docs for
> handling regressions[1].
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/Documentation/process/handling-regressions.rst?id=eed892da9cd08be76a8f467c600ef58716dbb4d2
> 

I'd rather Chuck make the final call here. The original patch
description didn't point out how easy it is to trigger a panic with
this, so I was hoping to convince him.

To further that argument too:

I have to wonder if this bug might cause (temporary?) memory corruption
on x86_64. The code hits a spinlock in that struct, so there may be a
window of time where it doesn't contain what's expected.

> > > > Cc: stable@...r.kernel.org # v6.3+
> > > > Fixes: f5f9d4a314da ("nfsd: move reply cache initialization into nfsd startup")
> > > 
> > > Why both Fixes: and Cc: stable?
> > 
> > *shrug* : they mean different things. I can drop the Cc stable.
> 
> Please leave it, only a stable tag ensures backporting; a fixes tag
> alone is not enough. See [1] above or these recent messages from Greg:
> 
> https://lore.kernel.org/all/2023061137-algorithm-almanac-1337@gregkh/
> https://lore.kernel.org/all/2023060703-colony-shakily-3514@gregkh/
> 

Chuck and I also recently requested that the stable series not pick
patches automatically for fs/nfsd. This does need to be backported
though, so I cc'ed stable to make that clear.
-- 
Jeff Layton <jlayton@...nel.org>