linux-kernel - RE: [PATCH v2 3/3] hfs: fix to update ctime after rename

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: <ca759782695a9e2195d39730edf939cadbe7016d.camel@ibm.com>
Date: Wed, 28 May 2025 01:37:43 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "frank.li@...o.com" <frank.li@...o.com>,
        "glaubitz@...sik.fu-berlin.de"
	<glaubitz@...sik.fu-berlin.de>,
        "slava@...eyko.com" <slava@...eyko.com>
CC: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 3/3] hfs: fix to update ctime after rename

On Tue, 2025-05-27 at 15:46 -0700, Viacheslav Dubeyko wrote:
> On Mon, 2025-05-19 at 10:52 -0600, Yangtao Li wrote:
> > Similar to hfsplus, let's update file ctime after the rename
> > operation
> > in hfs_rename().
> > 
> 
> Frankly speaking, I don't quite follow why should we update ctime
> during the rename operation. Why do we need to do this? What is the
> justification of this?
> 
> And we still continue to operate by atime [1-4]. Should we do something
> with it?
> 
> Thanks,
> Slava.
> 
> [1]
> https://elixir.bootlin.com/linux/v6.15/source/fs/hfsplus/inode.c#L519  
> [2]
> https://elixir.bootlin.com/linux/v6.15/source/fs/hfsplus/inode.c#L562  
> [3]
> https://elixir.bootlin.com/linux/v6.15/source/fs/hfsplus/inode.c#L609  
> [4]
> https://elixir.bootlin.com/linux/v6.15/source/fs/hfsplus/inode.c#L644  
> 

Sorry, I mean these links:

[1] https://elixir.bootlin.com/linux/v6.15/source/fs/hfs/sysdep.c#L35
[2] https://elixir.bootlin.com/linux/v6.15/source/fs/hfs/sysdep.c#L36
[3] https://elixir.bootlin.com/linux/v6.15/source/fs/hfs/inode.c#L357
[4] https://elixir.bootlin.com/linux/v6.15/source/fs/hfs/inode.c#L368

Thanks,
Slava.

> > W/O patch(xfstest generic/003):
> > 
> >  +ERROR: access time has not been updated after accessing file1 first
> > time
> >  +ERROR: access time has not been updated after accessing file2
> >  +ERROR: access time has changed after modifying file1
> >  +ERROR: change time has not been updated after changing file1
> >  +ERROR: access time has not been updated after accessing file3
> > second time
> >  +ERROR: access time has not been updated after accessing file3 third
> > time
> > 
> > W/ patch(xfstest generic/003):
> > 
> >  +ERROR: access time has not been updated after accessing file1 first
> > time
> >  +ERROR: access time has not been updated after accessing file2
> >  +ERROR: access time has changed after modifying file1
> >  +ERROR: access time has not been updated after accessing file3
> > second time
> >  +ERROR: access time has not been updated after accessing file3 third
> > time
> > 
> > Signed-off-by: Yangtao Li <frank.li@...o.com>
> > ---
> >  fs/hfs/dir.c | 17 ++++++++++-------
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
> > index 86a6b317b474..756ea7b895e2 100644
> > --- a/fs/hfs/dir.c
> > +++ b/fs/hfs/dir.c
> > @@ -284,6 +284,7 @@ static int hfs_rename(struct mnt_idmap *idmap,
> > struct inode *old_dir,
> >  		      struct dentry *old_dentry, struct inode
> > *new_dir,
> >  		      struct dentry *new_dentry, unsigned int flags)
> >  {
> > +	struct inode *inode = d_inode(old_dentry);
> >  	int res;
> >  
> >  	if (flags & ~RENAME_NOREPLACE)
> > @@ -296,14 +297,16 @@ static int hfs_rename(struct mnt_idmap *idmap,
> > struct inode *old_dir,
> >  			return res;
> >  	}
> >  
> > -	res = hfs_cat_move(d_inode(old_dentry)->i_ino,
> > -			   old_dir, &old_dentry->d_name,
> > +	res = hfs_cat_move(inode->i_ino, old_dir, &old_dentry-
> > > d_name,
> >  			   new_dir, &new_dentry->d_name);
> > -	if (!res)
> > -		hfs_cat_build_key(old_dir->i_sb,
> > -				  (btree_key
> > *)&HFS_I(d_inode(old_dentry))->cat_key,
> > -				  new_dir->i_ino, &new_dentry-
> > > d_name);
> > -	return res;
> > +	if (res)
> > +		return res;
> > +
> > +	hfs_cat_build_key(old_dir->i_sb, (btree_key *)&HFS_I(inode)-
> > > cat_key,
> > +			  new_dir->i_ino, &new_dentry->d_name);
> > +	inode_set_ctime_current(inode);
> > +	mark_inode_dirty(inode);
> > +	return 0;
> >  }
> >  
> >  const struct file_operations hfs_dir_operations = {