linux-kernel - Re: [PATCH v3 2/3] fat: make ctime and mtime identical explicitly

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: <87czhitr13.fsf@mail.parknet.co.jp>
Date:   Fri, 15 Apr 2022 22:28:08 +0900
From:   OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:     Chung-Chiang Cheng <cccheng@...ology.com>
Cc:     linux-kernel@...r.kernel.org, kernel@...heng.net,
        shepjeng@...il.com
Subject: Re: [PATCH v3 2/3] fat: make ctime and mtime identical explicitly

Chung-Chiang Cheng <cccheng@...ology.com> writes:

> -	fat_truncate_time(dir, NULL, S_ATIME|S_MTIME);
> +	fat_truncate_time(dir, NULL, S_ATIME|S_CTIME|S_MTIME);

fat_truncate_time() updates i_ctime too. So S_CTIME should not be
necessary here. And I think this is better to use only S_MTIME to tell
this is the point of mtime update.

(And, in fat_truncate_time(), I think S_CTIME is not required, because
we ignore ctime change, isn't it?)

Or you are going to update mtime on rename, etc too?

> +	/*
> +	 * ctime and mtime share the same on-disk field, and should be
> +	 * identical in memory.
> +	 */
> +	if (flags & (S_CTIME|S_MTIME)) {
>  		fat_truncate_mtime(sbi, now, &inode->i_mtime);
> +		inode->i_ctime = inode->i_mtime;
> +	}

[...]

>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_CTIME);
> +	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);

This is the point to update ctime. You want to affect ctime change to
mtime? As I said in previous post, I think we are better to ignore ctime
change, because it may become yet another incompatible behavior.

>  	fat_detach(inode);
>  out:
>  	mutex_unlock(&MSDOS_SB(sb)->s_lock);
> @@ -415,7 +415,7 @@ static int msdos_unlink(struct inode *dir, struct dentry *dentry)
>  	if (err)
>  		goto out;
>  	clear_nlink(inode);
> -	fat_truncate_time(inode, NULL, S_CTIME);
> +	fat_truncate_time(inode, NULL, S_CTIME|S_MTIME);

ditto

>  	fat_detach(inode);
>  out:
>  	mutex_unlock(&MSDOS_SB(sb)->s_lock);
> @@ -550,7 +550,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
>  		drop_nlink(new_inode);
>  		if (is_dir)
>  			drop_nlink(new_inode);
> -		fat_truncate_time(new_inode, &ts, S_CTIME);
> +		fat_truncate_time(new_inode, &ts, S_CTIME|S_MTIME);

ditto

>  	}
>  out:

[...]

> @@ -981,7 +981,7 @@ static int vfat_rename(struct user_namespace *mnt_userns, struct inode *old_dir,
>  		drop_nlink(new_inode);
>  		if (is_dir)
>  			drop_nlink(new_inode);
> -		fat_truncate_time(new_inode, &ts, S_CTIME);
> +		fat_truncate_time(new_inode, &ts, S_CTIME|S_MTIME);

ditto

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>