From: zhongjubin <[email protected]>
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 <[email protected]>
---
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
On Mon, Nov 15, 2021 at 07:00:18PM +0800, Jubin Zhong wrote:
> From: zhongjubin <[email protected]>
>
> 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.
> On Mon, Nov 15, 2021 at 07:00:18PM +0800, Jubin Zhong wrote:
>> From: zhongjubin <[email protected]>
>>
>> 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.
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.
> 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.
> 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.