linux-kernel - Re: [PATCH v4 8/8] nfsd: freeze c/mtime updates with outstanding WRITE

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: <CAN-5tyE1YazMhYJX5qT-YM=+h_rcQ+C9PZBNEEvwWRnWZtLXGQ@mail.gmail.com>
Date: Fri, 19 Dec 2025 12:43:41 -0500
From: Olga Kornievskaia <aglo@...ch.edu>
To: Jeff Layton <jlayton@...nel.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 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@...merspace.com>, 
	Anna Schumaker <anna@...nel.org>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, 
	linux-nfs@...r.kernel.org
Subject: Re: [PATCH v4 8/8] nfsd: freeze c/mtime updates with outstanding
 WRITE_ATTRS delegation

On Fri, Dec 19, 2025 at 12:25 PM Jeff Layton <jlayton@...nel.org> wrote:
>
> On Fri, 2025-12-19 at 10:58 -0500, Olga Kornievskaia wrote:
> > Hi Jeff,
> >
> > I narrowed down the upstream failure for generic/215 and generic/407
> > to this commit.
> >
> > Let's consider first where the kernel is compiled with delegated
> > attributes off (but it also fails just the same if the delegated
> > attributes are compiled in).
> >
> > I don't understand why the code unconditionally changed to call
> > nfsd4_finalize_deleg_timestamps() which I think the main driver behind
> > the failure.
> >
> > Running generic/407 there is an OPEN (which gives out a write
> > delegation) and returns a change id, then on this filehandle there is
> > a SETATTR (with a getattr) which returns a new changeid. Then there is
> > a CLONE where the filehandle is the destination filehandle on which
> > there is a getattr which returns unchanged changeid/modify time (bad).
> > Then there is a DELEGRETURN (with a getattr) which again returns same
> > change id. Test fails.
> >
> > Prior to this commit. The changeid/modify time is different in CLONE
> > and DELEGRETURN -- test passes.
> >
> > Now let me describe what happens with delegated attributes enabled.
> > OPEN returns delegated attributes delegation, included getattr return
> > a changeid. Then CLONE is done, the included gettattr returns a
> > different (from open's) changeid (different time_modify). Then there
> > is SETATTR+GEATTR+DELEGRETURN compound from the client (which carries
> > a time_deleg_modify value different from above). Server in getattr
> > replies with changeid same as in clone and mtime with the value client
> > provided. So I'm not sure exactly why the test fails here but that's a
> > different problem as my focus is on "delegation attribute off option"
> > at the moment.
> >
> > I don't know if this is the correct fix or not but perhaps we
> > shouldn't unconditionally be setting this mode? (note this fix only
> > fixes the delegattributes off. however i have no claims that this
> > patch is what broke 215/407 for delegated attributes on. Something
> > else is in play there). If this solution is acceptable, I can send a
> > patch.
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 81fa7cc6c77b..624cc6ab2802 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -6318,7 +6318,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp,
> > struct nfsd4_open *open,
> >                 dp->dl_ctime = stat.ctime;
> >                 dp->dl_mtime = stat.mtime;
> >                 spin_lock(&f->f_lock);
> > -               f->f_mode |= FMODE_NOCMTIME;
> > +               if (deleg_ts)
> > +                       f->f_mode |= FMODE_NOCMTIME;
> >                 spin_unlock(&f->f_lock);
> >                 trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> >         } else {
> >
> >
>
> That patch does look correct to me -- nice catch. Have you validated
> that it fixes 215 and 407?

Yes, it does fix 215 and 407 for me.

>
> Thanks,
> Jeff
> --
> Jeff Layton <jlayton@...nel.org>