From b45fbe0bbba45691258fe276c406569f9f8c1b8c Mon Sep 17 00:00:00 2001 From: Auke Kok Date: Mon, 9 Sep 2024 23:23:11 -0400 Subject: [PATCH] Don't pass data version to attr_x unless the ioctl means to set it. The wrapper in setattr_more that translates the operations to attr_x needs to decide whether to ask attr_x to perform a change to any of the fields passed to it or not. For the date and size fields this is implicit - we always tell attr_x to change them. For any of the other fields, it should be explicit. The only field that is in the struct that this applies to is data_version. Because the data version field by default is zero, we use that as condition to decide whether to pass the data_version down to attr_x. Previously, the code would always pass a data_version=0 down to attr_x, triggering one of the validity checks, making it return -EINVAL. We add a simple test case to test for this issue. Signed-off-by: Auke Kok --- kmod/src/ioctl.c | 7 +++++-- tests/golden/setattr_more | 1 + tests/tests/setattr_more.sh | 7 +++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/kmod/src/ioctl.c b/kmod/src/ioctl.c index cbf442bd..45fc0e51 100644 --- a/kmod/src/ioctl.c +++ b/kmod/src/ioctl.c @@ -674,8 +674,8 @@ static long scoutfs_ioc_setattr_more(struct file *file, unsigned long arg) goto out; } - iax->x_mask = SCOUTFS_IOC_IAX_DATA_VERSION | SCOUTFS_IOC_IAX_CTIME | - SCOUTFS_IOC_IAX_CRTIME | SCOUTFS_IOC_IAX_SIZE; + iax->x_mask = SCOUTFS_IOC_IAX_CTIME | SCOUTFS_IOC_IAX_CRTIME | + SCOUTFS_IOC_IAX_SIZE; iax->data_version = sm.data_version; iax->ctime_sec = sm.ctime_sec; iax->ctime_nsec = sm.ctime_nsec; @@ -686,6 +686,9 @@ static long scoutfs_ioc_setattr_more(struct file *file, unsigned long arg) if (sm.flags & SCOUTFS_IOC_SETATTR_MORE_OFFLINE) iax->x_flags |= SCOUTFS_IOC_IAX_F_SIZE_OFFLINE; + if (sm.data_version != 0) + iax->x_mask |= SCOUTFS_IOC_IAX_DATA_VERSION; + ret = mnt_want_write_file(file); if (ret < 0) goto out; diff --git a/tests/golden/setattr_more b/tests/golden/setattr_more index 040ffe07..7a8bfbe1 100644 --- a/tests/golden/setattr_more +++ b/tests/golden/setattr_more @@ -29,3 +29,4 @@ File size of /mnt/test/test/setattr_more/file is 40988672 (10007 blocks of 4096 /mnt/test/test/setattr_more/file: 1 extent found == correct offline extent length 976563 +== omitting data_version should not fail diff --git a/tests/tests/setattr_more.sh b/tests/tests/setattr_more.sh index c15a507a..846118ab 100644 --- a/tests/tests/setattr_more.sh +++ b/tests/tests/setattr_more.sh @@ -75,4 +75,11 @@ scoutfs setattr -V 1 -o -s 4000000000 "$FILE" 2>&1 | t_filter_fs scoutfs stat -s offline_blocks "$FILE" rm "$FILE" +# Do not fail if data_version is unset - the unset `0` value should not +# be passed down to attr_x handling code which will -EINVAL on that. +echo "== omitting data_version should not fail" +touch "$FILE" +scoutfs setattr -s 0 -t 1725670311.0 -r 1725670311.0 "$FILE" +rm "$FILE" + t_pass