linux-kernel - Re: [PATCH] nfsd: don't set the ctime on delegated atime updates

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: <c3f4b22f65aa76c6d77becdf16174c8f55845fec.camel@kernel.org>
Date: Tue, 15 Jul 2025 13:33:13 -0400
From: Jeff Layton <jlayton@...nel.org>
To: 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>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfsd: don't set the ctime on delegated atime updates

On Tue, 2025-07-15 at 12:46 -0400, Chuck Lever wrote:
> On 7/15/25 7:34 AM, Jeff Layton wrote:
> > Clients will typically precede a DELEGRETURN for a delegation with
> > delegated timestamp with a SETATTR to set the timestamps on the server
> > to match what the client has.
> > 
> > knfsd implements this by using the nfsd_setattr() infrastructure, which
> > will set ATTR_CTIME on any update that goes to notify_change(). This is
> > problematic as it means that the client will get a spurious ctime
> > updates when updating the atime.
> > 
> > Fix this by pushing the handling of ATTR_CTIME down into the decoder
> > functions so that they are set earlier and more deliberately, and stop
> > nfsd_setattr() from implicitly adding it to every notify_change() call.
> > 
> > Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> I was concerned about the description making a claim without reference
> that modifying atime does not modify ctime. That claim has been somewhat
> contentious in the past.
> 
> Search engines suggest that IEEE 1003.1 does indeed specify that an
> atime modification does not require a ctime change. POSIX does not make
> these standards free, however. Maybe a Linux-specific document has
> something on point.
> 

Ok, good. We probably ought to spell this out in some manpage...

> Even so, I'm comfortable taking this for now and putting it through the
> normal set of regression testing.
> 
> Thank you for pursuing this one, Jeff!
> 

Thanks. I think the principle is correct. Unfortunately, this patch
misses a place where we need to set ATTR_CTIME. You may want to drop it
for now.

I'm working on a different approach that makes it skip setting
ATTR_CTIME if ATTR_DELEG is set. ATTR_DELEG gets set on any delegated
timestamp update, so that should make it do the right thing (and the
patch will be less invasive).

Thanks for the review so far!
-- 
Jeff Layton <jlayton@...nel.org>