Re: [LTP] [PATCH v5] syscalls/mount_setattr01: Add basic functional test

Petr Vorel
2022-05-18T08:51:08Z

Hi Chen, Dai,

...
> +static void setup(void)
> +{
> + fsopen_supported_by_kernel();
> + struct stat st = {0};
> +
> + if (stat(OT_MNTPOINT, &st) == -1)
> + SAFE_MKDIR(OT_MNTPOINT, 0777);
> +}
> +
> +static void run(unsigned int n)
> +{
> + struct tcase *tc = &tcases[n];
> + struct mount_attr attr = {
> + .attr_set = tc->mount_attrs,
> + };
> + struct statvfs buf;
> +
> + TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> + AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> + if (!TST_PASS)
> + return;
> +
> + otfd = (int)TST_RET;
> +
> + TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
> + "%s set", tc->name);
> + if (!TST_PASS)
> + goto out1;
> +
> + TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
> + if (!TST_PASS)
> + goto out1;
> + mount_flag = 1;
> + SAFE_CLOSE(otfd);
> +
> + TST_EXP_PASS_SILENT(statvfs(OT_MNTPOINT, &buf), "statvfs sucess");
> + if (!TST_PASS)
> + goto out2;
> +
> + if (buf.f_flag & tc->expect_attrs)
> + tst_res(TPASS, "%s is actually set as expected", tc->name);
> + else
> + tst_res(TFAIL, "%s is not actually set as expected", tc->name);
> +
> + goto out2;
> +
> +out1:
> + SAFE_CLOSE(otfd);
> +out2:
> + mount_flag = 0;
> + SAFE_UMOUNT(OT_MNTPOINT);
mount_flag needs to be checked before otherwise if it fails on:
TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
"%s set", tc->name);

It tries to umount() not yet mounted filesystem:

mount_setattr01.c:109: TPASS: MOUNT_ATTR_RDONLY is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NOSUID is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NODEV is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NOEXEC is actually set as expected
mount_setattr01.c:93: TFAIL: MOUNT_ATTR_NOSYMFOLLOW set failed: EINVAL (22)
tst_device.c:394: TWARN: umount('ot_mntpoint') failed with EINVAL
mount_setattr01.c:119: TBROK: umount(ot_mntpoint) failed: EINVAL (22)

Because we mount later than opening otfd with open_tree() but need to close otfd
before umount() we cannot depend only on labels.

Thus I suggest to have this cleanup code:

out1:
SAFE_CLOSE(otfd);
out2:
if (mount_flag)
SAFE_UMOUNT(OT_MNTPOINT);

mount_flag = 0;

If Cyril is ok with this I'd merge it before release.
(I tested it on various systems, found only this issue.)

Kind regards,
Petr

+++ testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
@@ -115,8 +115,10 @@ static void run(unsigned int n)
out1:
SAFE_CLOSE(otfd);
out2:
+ if (mount_flag)
+ SAFE_UMOUNT(OT_MNTPOINT);
+
mount_flag = 0;
- SAFE_UMOUNT(OT_MNTPOINT);
}

static struct tst_test test = {