[LTP] [PATCH v4] syscalls/statx04: use stx_attributes_mask before test
Richard Palethorpe
rpalethorpe@suse.de
Fri Nov 12 11:21:56 CET 2021
Hello Yang,
Yang Xu <xuyang2018.jy@fujitsu.com> writes:
> stx_attributes_mask shows what's supported in stx_attributes.
> Set supp_{append,compr,immutable,nodump} attributes only on filesystems
> which actually support it.
>
> Also merge duplicate code and document the kernel commits that ext2/ext4/xfs/btrfs
> supports statx syscall correctly.
>
> Reviewed-by: Li Wang <liwang@redhat.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
> v3->v4:
> 1. add kernel commits into top description
> 2. docparse formatting
Note that I also suggested NOT checking the kernel version. Because
these patches are likely to get backported.
Please see comments below.
>
> v2->v3:
> 1.add kernel version check for stx_attributes_mask
> 2. use test_flag(int) instead of test_flagged and test_unflagged
>
> testcases/kernel/syscalls/statx/statx04.c | 197 ++++++++++++++--------
> 1 file changed, 123 insertions(+), 74 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> index f8350ed2d..3c221b461 100644
> --- a/testcases/kernel/syscalls/statx/statx04.c
> +++ b/testcases/kernel/syscalls/statx/statx04.c
> @@ -4,21 +4,46 @@
> * Email: code@zilogic.com
> */
>
> -/*
> - * Test statx
> +/*\
> + * [Description]
> *
> * This code tests if the attributes field of statx received expected value.
> * File set with following flags by using SAFE_IOCTL:
> - * 1) STATX_ATTR_COMPRESSED - The file is compressed by the filesystem.
> - * 2) STATX_ATTR_IMMUTABLE - The file cannot be modified.
> - * 3) STATX_ATTR_APPEND - The file can only be opened in append mode for
> - * writing.
> - * 4) STATX_ATTR_NODUMP - File is not a candidate for backup when a backup
> + *
> + * - STATX_ATTR_COMPRESSED - The file is compressed by the filesystem.
> + * - STATX_ATTR_IMMUTABLE - The file cannot be modified.
> + * - STATX_ATTR_APPEND - The file can only be opened in append mode for writing.
> + * - STATX_ATTR_NODUMP - File is not a candidate for backup when a backup
> * program such as dump(8) is run.
> *
> * Two directories are tested.
> - * First directory has all flags set.
> - * Second directory has no flags set.
> + * First directory has all flags set. Second directory has no flags set.
> + *
> + * ext2, ext4, btrfs and xfs support statx syscall since the following commit
> + *
> + * commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> + * Author: yangerkun <yangerkun@huawei.com>
> + * Date: Mon Feb 18 09:07:02 2019 +0800
> + *
> + * ext2: support statx syscall
> + *
> + * commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
> + * Author: David Howells <dhowells@redhat.com>
> + * Date: Fri Mar 31 18:31:56 2017 +0100
> + *
> + * ext4: Add statx support
> + *
> + * commit 04a87e3472828f769a93655d7c64a27573bdbc2c
> + * Author: Yonghong Song <yhs@fb.com>
> + * Date: Fri May 12 15:07:43 2017 -0700
> + *
> + * Btrfs: add statx support
> + *
> + * commit 1b9598c8fb9965fff901c4caa21fed9644c34df3
> + * Author: Luis R. Rodriguez <mcgrof@kernel.org>
> + * Date: Fri Mar 1 08:14:57 2019 -0800
> + *
> + * xfs: fix reporting supported extra file attributes for statx()
This is good. However I also meant that you can add the commit tag to
tst_test. E.g. from the cve-2014-0196 testcase
static struct tst_test test = {
.setup = setup,
.cleanup = cleanup,
.test_all = run,
.tags = (const struct tst_tag[]) {
{"linux-git", "4291086b1f08"},
{"CVE", "2014-0196"},
{}
}
};
> *
> * Minimum kernel version required is 4.11.
> */
> @@ -34,85 +59,67 @@
> #define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
>
> static int fd, clear_flags;
> +static int supp_compr = 1, supp_append = 1, supp_immutable = 1, supp_nodump = 1;
>
> -static void test_flagged(void)
> -{
> - struct statx buf;
> -
> - TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
> - if (TST_RET == 0)
> - tst_res(TPASS,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> - else
> - tst_brk(TFAIL | TTERRNO,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> -
> - if (buf.stx_attributes & STATX_ATTR_COMPRESSED)
> - tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is not set");
> -
> - if (buf.stx_attributes & STATX_ATTR_APPEND)
> - tst_res(TPASS, "STATX_ATTR_APPEND flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_APPEND flag is not set");
> -
> - if (buf.stx_attributes & STATX_ATTR_IMMUTABLE)
> - tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is not set");
> -
> - if (buf.stx_attributes & STATX_ATTR_NODUMP)
> - tst_res(TPASS, "STATX_ATTR_NODUMP flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_NODUMP flag is not set");
> -}
> -
> -static void test_unflagged(void)
> +static void test_flag(int flag)
> {
> struct statx buf;
>
> - TEST(statx(AT_FDCWD, TESTDIR_UNFLAGGED, 0, 0, &buf));
> + TEST(statx(AT_FDCWD, flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED, 0, 0, &buf));
> if (TST_RET == 0)
> tst_res(TPASS,
> "sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> - TESTDIR_UNFLAGGED);
> + flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED);
> else
> tst_brk(TFAIL | TTERRNO,
> "sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> - TESTDIR_UNFLAGGED);
> -
> - if ((buf.stx_attributes & STATX_ATTR_COMPRESSED) == 0)
> - tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_APPEND) == 0)
> - tst_res(TPASS, "STATX_ATTR_APPEND flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_APPEND flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_IMMUTABLE) == 0)
> - tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_NODUMP) == 0)
> - tst_res(TPASS, "STATX_ATTR_NODUMP flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_NODUMP flag is set");
> + flag ? TESTDIR_FLAGGED : TESTDIR_UNFLAGGED);
> +
> + if (supp_compr) {
> + if (buf.stx_attributes & STATX_ATTR_COMPRESSED)
> + tst_res(flag ? TPASS : TFAIL,
> + "STATX_ATTR_COMPRESSED flag is set");
> + else
> + tst_res(flag ? TFAIL : TPASS,
> + "STATX_ATTR_COMPRESSED flag is not set");
> + }
> + if (supp_append) {
> + if (buf.stx_attributes & STATX_ATTR_APPEND)
> + tst_res(flag ? TPASS : TFAIL,
> + "STATX_ATTR_APPEND flag is set");
> + else
> + tst_res(flag ? TFAIL : TPASS,
> + "STATX_ATTR_APPEND flag is not set");
> + }
> + if (supp_immutable) {
> + if (buf.stx_attributes & STATX_ATTR_IMMUTABLE)
> + tst_res(flag ? TPASS : TFAIL,
> + "STATX_ATTR_IMMUTABLE flag is set");
> + else
> + tst_res(flag ? TFAIL : TPASS,
> + "STATX_ATTR_IMMUTABLE flag is not set");
> + }
> + if (supp_nodump) {
> + if (buf.stx_attributes & STATX_ATTR_NODUMP)
> + tst_res(flag ? TPASS : TFAIL,
> + "STATX_ATTR_NODUMP flag is set");
> + else
> + tst_res(flag ? TFAIL : TPASS,
> + "STATX_ATTR_NODUMP flag is not set");
> + }
> }
>
> struct test_cases {
> - void (*tfunc)(void);
> + void (*tfunc)(int);
> + int set_flag;
> } tcases[] = {
> - {&test_flagged},
> - {&test_unflagged},
> + {&test_flag, 1},
> + {&test_flag, 0},
> };
>
> static void run(unsigned int i)
> {
> - tcases[i].tfunc();
> + tcases[i].tfunc(tcases[i].set_flag);
> }
>
> static void caid_flags_setup(void)
> @@ -135,12 +142,17 @@ static void caid_flags_setup(void)
> tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_GETFLAGS, ...)", fd);
> }
>
> - attr |= FS_COMPR_FL | FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL;
> + if (supp_compr)
> + attr |= FS_COMPR_FL;
> + if (supp_append)
> + attr |= FS_APPEND_FL;
> + if (supp_immutable)
> + attr |= FS_IMMUTABLE_FL;
> + if (supp_nodump)
> + attr |= FS_NODUMP_FL;
>
> ret = ioctl(fd, FS_IOC_SETFLAGS, &attr);
> if (ret < 0) {
> - if (errno == EOPNOTSUPP)
> - tst_brk(TCONF, "Flags not supported");
> tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
> }
>
> @@ -149,11 +161,48 @@ static void caid_flags_setup(void)
>
> static void setup(void)
> {
> + struct statx buf;
> +
> SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
> SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
So we create the dirs here. I assume they are created with all blank
flags otherwise the existing test would be broken.
>
> + /* Don't check ext4 because ext4 supports statx since 4.11. */
> if (!strcmp(tst_device->fs_type, "btrfs") && tst_kvercmp(4, 13, 0) < 0)
> - tst_brk(TCONF, "Btrfs statx() supported since 4.13");
> + tst_brk(TCONF,
> + "Btrfs statx() stx_attributes_mask supported since 4.13, see test description!");
> +
> + if (!strcmp(tst_device->fs_type, "xfs") && tst_kvercmp(5, 1, 0) < 0)
> + tst_brk(TCONF,
> + "xfs statx() stx_attributes_mask supported since 5.1, see test description!");
> +
> + if (!strcmp(tst_device->fs_type, "ext2") && tst_kvercmp(5, 1, 0) < 0)
> + tst_brk(TCONF,
> + "ext2 statx() stx_attributes_mask supported since 5.1, see test description!");
> +
> + TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
Then we get the flags here. They should all be blank.
> + if (TST_RET == -1)
> + tst_brk(TFAIL | TTERRNO,
> + "sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> +
> + if ((buf.stx_attributes_mask & FS_COMPR_FL) == 0) {
> + supp_compr = 0;
> + tst_res(TCONF, "filesystem doesn't support FS_COMPR_FL");
> + }
> + if ((buf.stx_attributes_mask & FS_APPEND_FL) == 0) {
> + supp_append = 0;
> + tst_res(TCONF, "filesystem doesn't support FS_APPEND_FL");
> + }
> + if ((buf.stx_attributes_mask & FS_IMMUTABLE_FL) == 0) {
> + supp_immutable = 0;
> + tst_res(TCONF, "filesystem doesn't support FS_IMMUTABLE_FL");
> + }
> + if ((buf.stx_attributes_mask & FS_NODUMP_FL) == 0) {
> + supp_nodump = 0;
> + tst_res(TCONF, "filesystem doesn't support FS_NODUMP_FL");
> + }
> + if (!(supp_compr || supp_append || supp_immutable || supp_nodump))
> + tst_brk(TCONF,
> + "filesystem doesn't support the above any attr,
> skip it");
So this will always fail with TCONF?
>
> caid_flags_setup();
And then we set the flags here?
Note that even if you move setting the flags to the top. This will still
result in all unset flags becoming TCONF instead of TFAIL.
IMO statx is broken on older kernels except for ext4. It can be fixed by
backporting patches. One of the main use cases for LTP is to find
missing backports.
--
Thank you,
Richard.
More information about the ltp
mailing list