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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ca86b70c1a4a25c2f084bfce53ed864a557ebfed.camel@kernel.org>
Date: Fri, 19 Dec 2025 12:25:10 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Olga Kornievskaia <aglo@...ch.edu>
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, 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?

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