* [PATCH] fs: Fix truncate never updates m/ctime
@ 2021-11-15 11:00 Jubin Zhong
2021-11-15 17:58 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Jubin Zhong @ 2021-11-15 11:00 UTC (permalink / raw)
To: viro; +Cc: wangfangpeng1, kechengsong, linux-fsdevel, linux-kernel
From: zhongjubin <zhongjubin@huawei.com>
Syscall truncate() never updates m/ctime even if the file size is
changed. However, this is incorrect according to man file:
truncate (2):
If the size changed, then the st_ctime and st_mtime fields
(respectively, time of last status change and time of last modification;
see stat(2)) for the file are updated, and the set-user-ID and
set-group-ID mode bits may be cleared.
Check file size before do_truncate() to fix this.
Signed-off-by: zhongjubin <zhongjubin@huawei.com>
---
fs/open.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index f732fb94600c..02404b759c2e 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -106,8 +106,11 @@ long vfs_truncate(const struct path *path, loff_t length)
goto put_write_and_out;
error = security_path_truncate(path);
- if (!error)
- error = do_truncate(mnt_userns, path->dentry, length, 0, NULL);
+ if (error)
+ goto put_write_and_out;
+
+ if (i_size_read(inode) != length)
+ error = do_truncate(mnt_userns, path->dentry, length, ATTR_MTIME | ATTR_CTIME, NULL);
put_write_and_out:
put_write_access(inode);
--
2.12.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Fix truncate never updates m/ctime
2021-11-15 11:00 [PATCH] fs: Fix truncate never updates m/ctime Jubin Zhong
@ 2021-11-15 17:58 ` Christoph Hellwig
2021-11-16 3:58 ` Jubin Zhong
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-11-15 17:58 UTC (permalink / raw)
To: Jubin Zhong; +Cc: viro, wangfangpeng1, kechengsong, linux-fsdevel, linux-kernel
On Mon, Nov 15, 2021 at 07:00:18PM +0800, Jubin Zhong wrote:
> From: zhongjubin <zhongjubin@huawei.com>
>
> Syscall truncate() never updates m/ctime even if the file size is
> changed. However, this is incorrect according to man file:
>
> truncate (2):
> If the size changed, then the st_ctime and st_mtime fields
> (respectively, time of last status change and time of last modification;
> see stat(2)) for the file are updated, and the set-user-ID and
> set-group-ID mode bits may be cleared.
>
> Check file size before do_truncate() to fix this.
Please try to actually reproduce your alleged "bug". And maybe also
look at the actual setattr implementations. Hint: The XFS one even
has extensive comments.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Fix truncate never updates m/ctime
2021-11-15 17:58 ` Christoph Hellwig
@ 2021-11-16 3:58 ` Jubin Zhong
2021-11-16 5:22 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Jubin Zhong @ 2021-11-16 3:58 UTC (permalink / raw)
To: hch
Cc: kechengsong, linux-fsdevel, linux-kernel, viro, wangfangpeng1,
zhongjubin
> On Mon, Nov 15, 2021 at 07:00:18PM +0800, Jubin Zhong wrote:
>> From: zhongjubin <zhongjubin@huawei.com>
>>
>> Syscall truncate() never updates m/ctime even if the file size is
>> changed. However, this is incorrect according to man file:
>>
>> truncate (2):
>> If the size changed, then the st_ctime and st_mtime fields
>> (respectively, time of last status change and time of last modification;
>> see stat(2)) for the file are updated, and the set-user-ID and
>> set-group-ID mode bits may be cleared.
>>
>> Check file size before do_truncate() to fix this.
>
> Please try to actually reproduce your alleged "bug". And maybe also
> look at the actual setattr implementations. Hint: The XFS one even
> has extensive comments.
Thanks for your advice. I found this problem on yaffs2 in the beginning,
ftruncate() always works fine but truncate() does not. Now I have done
a few more tests and the following are the results:
Test Environmont:
kernel: Linux Kernel v5.16
hardware: QEMU emulator version 3.1.0
arch: vexpress-v2p-ca9
Teset Results:
filesystems m/ctime updated by truncate?
jffs2 fail
yaffs2 fail
ubifs success
ext2 success
ext4 success
tmpfs success
xfs success
Test Steps:
1. cd /path/to/mnt/point
2. dd if=/dev/zero of=test bs=1M count=1
3. stat test
4. /bin/my_truncate -s 1024 test
5. stat test
6. compare m/ctime of step 5 with step 3
Program source:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
int main(int argc, char **argv)
{
int ret;
char file_name[128] = {0};
if (argc < 4 || argv == NULL || argv[1] == NULL || argv[2] == NULL || argv[3] == NULL) {
return -1;
}
if (strcmp(argv[1], "-s")) {
return -1;
}
if (realpath(argv[3], file_name) == NULL) {
printf("truncate: input file name %s err.\n", argv[3]);
return -1;
}
off_t size = (off_t)strtol(argv[2], 0, 0);
ret = truncate(file_name, size);
if (ret) {
printf("truncate return err %d\n", ret);
}
return ret;
}
I work on embedded devices so concern about jffs2/yaffs2/ubifs the most.
If there are any errors in my test program please let me know.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Fix truncate never updates m/ctime
2021-11-16 3:58 ` Jubin Zhong
@ 2021-11-16 5:22 ` Christoph Hellwig
2021-11-16 6:57 ` Jubin Zhong
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-11-16 5:22 UTC (permalink / raw)
To: Jubin Zhong
Cc: hch, kechengsong, linux-fsdevel, linux-kernel, viro, wangfangpeng1
On Tue, Nov 16, 2021 at 11:58:10AM +0800, Jubin Zhong wrote:
> I work on embedded devices so concern about jffs2/yaffs2/ubifs the most.
> If there are any errors in my test program please let me know.
It seems like you need to fix jffs2 to implement the proper semantics
in its ->setattr.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Fix truncate never updates m/ctime
2021-11-16 5:22 ` Christoph Hellwig
@ 2021-11-16 6:57 ` Jubin Zhong
2021-11-16 7:02 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Jubin Zhong @ 2021-11-16 6:57 UTC (permalink / raw)
To: hch
Cc: kechengsong, linux-fsdevel, linux-kernel, viro, wangfangpeng1,
zhongjubin
> It seems like you need to fix jffs2 to implement the proper semantics in its ->setattr.
Yes I have thought of this solution. However, when I tried to
track this problem down, I found that ftruncate() had similar
problem and it was fixed by commit 6e656be89999 ("ftruncate
does not always update m/ctime"):
diff --git a/fs/open.c b/fs/open.c
index 5fb16e5267dc..303f06d2a7b9 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -322,7 +322,7 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
error = locks_verify_truncate(inode, file, length);
if (!error)
- error = do_truncate(dentry, length, 0, file);
+ error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
out_putf:
fput(file);
out:
In my opinion, there are two advantages if we fix it in
vfs_truncate():
1. All filesystems can reuse the scheme without adapting
Separately, just like what we did for ftruncate().
2. In the case when old_size = new_size, we can avoid
calling do_truncate() and return without doing anything.
Hope that you can consider my suggestion, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: Fix truncate never updates m/ctime
2021-11-16 6:57 ` Jubin Zhong
@ 2021-11-16 7:02 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-11-16 7:02 UTC (permalink / raw)
To: Jubin Zhong
Cc: hch, kechengsong, linux-fsdevel, linux-kernel, viro, wangfangpeng1
> In my opinion, there are two advantages if we fix it in
> vfs_truncate():
Please actually read the comments in the xfs setattr implementation
I pointed you to first time.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-16 7:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 11:00 [PATCH] fs: Fix truncate never updates m/ctime Jubin Zhong
2021-11-15 17:58 ` Christoph Hellwig
2021-11-16 3:58 ` Jubin Zhong
2021-11-16 5:22 ` Christoph Hellwig
2021-11-16 6:57 ` Jubin Zhong
2021-11-16 7:02 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).