[LTP] [PATCH v3 0/2] mount03: Convert to new API
xuyang2018.jy@fujitsu.com
xuyang2018.jy@fujitsu.com
Mon Aug 15 07:15:20 CEST 2022
Hi Petr
> Hi,
>
> I wanted to speedup mount03 rewrite [1], thus I finished the work.
>
> Changes include:
> * simplify code by using:
> - SAFE macros
> - TST_EXP_FAIL() instead of TST_EXP_FAIL2()
> - remove if () check from SAFE macros (that's the point of safe macros
> to not having to use if () check
>
> * fix mount03_setuid_test call, so it can expect 0 exit code
> I wonder myself why this fixes it:
> - SAFE_SETREUID(nobody_uid, nobody_gid);
Why here is nobody_gid?
> + SAFE_SETGID(nobody_gid);
> + SAFE_SETREUID(-1, nobody_uid);
What problem do you meet?
>
> * add missing TST_RESOURCE_COPY()
> @Cyril: is it really needed?
IMO, if we use resourcein test struct, we don't need it because ltp lib
has move it to tmpdir, we can just use SAFE_COPY ie prctl06.c.
>
> * do not run test function if initial mount() fails
>
> * cleanup useless comments
> * style changes (simplify function and variable names, simplify docparse)
>
> Full diff is below.
>
> Kind regards,
> Petr
>
> [1] https://lore.kernel.org/ltp/20220726070206.266-1-chenhx.fnst@fujitsu.com/
>
> Petr Vorel (1):
> tst_test_macros.h: Add TST_EXP_EQ_STR
>
> chenhx.fnst@fujitsu.com (1):
> mount03: Convert to new API
>
> include/tst_test_macros.h | 10 +
> testcases/kernel/syscalls/mount/mount03.c | 495 +++++++---------------
> 2 files changed, 164 insertions(+), 341 deletions(-)
>
> diff --git include/tst_test_macros.h include/tst_test_macros.h
> index c8f7825c4..8cc959243 100644
> --- include/tst_test_macros.h
> +++ include/tst_test_macros.h
> @@ -242,4 +242,14 @@ extern void *TST_RET_PTR;
> #define TST_EXP_EQ_SSZ(VAL_A, VAL_B) \
> TST_EXP_EQ_(VAL_A, #VAL_A, VAL_B, #VAL_B, ssize_t, "%zi")
>
> +#define TST_EXP_EQ_STR(STR_A, STR_B) do {\
> + if (strcmp(STR_A, STR_B)) { \
> + tst_res_(__FILE__, __LINE__, TFAIL, \
> + "'%s' != '%s'", STR_A, STR_B); \
> + } else { \
> + tst_res_(__FILE__, __LINE__, TPASS, \
> + "'%s' == '%s'", STR_A, STR_B); \
> + } \
> +} while (0)
> +
> #endif /* TST_TEST_MACROS_H__ */
> diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> index e6395c592..74b018d78 100644
> --- testcases/kernel/syscalls/mount/mount03.c
> +++ testcases/kernel/syscalls/mount/mount03.c
> @@ -7,265 +7,176 @@
> /*\
> * [Description]
> *
> - * Check for basic mount(2) system call flags.
> - *
> - * Verify that mount(2) syscall passes for each flag setting and validate
> - * the flags
> - *
> - * - MS_RDONLY - mount read-only.
> - * - MS_NODEV - disallow access to device special files.
> - * - MS_NOEXEC - disallow program execution.
> - * - MS_SYNCHRONOUS - writes are synced at once.
> - * - MS_REMOUNT - alter flags of a mounted FS.
> - * - MS_NOSUID - ignore suid and sgid bits.
> - * - MS_NOATIME - do not update access times.
> + * Verify mount(2) for various flags.
> */
>
> -#ifndef _GNU_SOURCE
> -#define _GNU_SOURCE
> -#endif
> -
> -#define TEMP_FILE "temp_file"
> -#define FILE_MODE 0644
> -#define SUID_MODE 0511
> -
> #include <stdio.h>
> +#include <stdlib.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> -#include <linux/limits.h>
> -#include <stdlib.h>
> #include <pwd.h>
> +#include "old_resource.h"
> #include "tst_test.h"
> -#include "tst_safe_file_ops.h"
> #include "lapi/mount.h"
>
> #define MNTPOINT "mntpoint"
> #define TESTBIN "mount03_setuid_test"
> -#define TCASE_ENTRY(_flags, _do_test) \
> - { \
> - .name = "Flag " #_flags, \
> - .flags = _flags, \
> - .do_test = _do_test \
> - }
> +#define TEST_STR "abcdefghijklmnopqrstuvwxyz"
> +#define FILE_MODE 0644
> +#define SUID_MODE 0511
>
> static int otfd;
> -static char write_buffer[BUFSIZ];
> -static char read_buffer[BUFSIZ];
> +static char wbuf[BUFSIZ];
> +static char rbuf[BUFSIZ];
> static char file[PATH_MAX];
> static uid_t nobody_uid;
> static gid_t nobody_gid;
>
> -static void test_ms_nosuid(void);
> -static void test_ms_rdonly(void);
> -static void test_ms_nodev(void);
> -static void test_noexec(void);
> -static void test_ms_synchronous(void);
> -static void test_ms_remount(void);
> -static void test_ms_noatime(void);
> -
> -static struct tcase {
> - char *name;
> - unsigned int flags;
> - void (*do_test)(void);
> -} tcases[] = {
> - TCASE_ENTRY(MS_RDONLY, test_ms_rdonly),
> - TCASE_ENTRY(MS_NODEV, test_ms_nodev),
> - TCASE_ENTRY(MS_NOEXEC, test_noexec),
> - TCASE_ENTRY(MS_SYNCHRONOUS, test_ms_synchronous),
> - TCASE_ENTRY(MS_RDONLY, test_ms_remount),
> - TCASE_ENTRY(MS_NOSUID, test_ms_nosuid),
> - TCASE_ENTRY(MS_NOATIME, test_ms_noatime),
> -};
> +static void test_rdonly(void)
> +{
> + snprintf(file, PATH_MAX, "%s/rdonly", MNTPOINT);
> + TST_EXP_FAIL(otfd = open(file, O_CREAT | O_RDWR, 0700), EROFS);
> +}
>
> -static void test_ms_rdonly(void)
> +static void test_nodev(void)
> {
> - /* Validate MS_RDONLY flag of mount call */
> + snprintf(file, PATH_MAX, "%s/nodev", MNTPOINT);
> + SAFE_MKNOD(file, S_IFBLK | 0777, 0);
> + TST_EXP_FAIL(otfd = open(file, O_RDWR, 0700), EACCES);
> + SAFE_UNLINK(file);
> +}
>
> - snprintf(file, PATH_MAX, "%s/tmp", MNTPOINT);
> - TST_EXP_FAIL2(open(file, O_CREAT | O_RDWR, 0700),
> - EROFS, "mount(2) passed with flag MS_RDONLY: "
> - "open fail with EROFS as expected");
> +static void test_noexec(void)
> +{
> + snprintf(file, PATH_MAX, "%s/noexec", MNTPOINT);
> + otfd = SAFE_OPEN(file, O_CREAT | O_RDWR, 0700);
> + TST_EXP_FAIL(execlp(file, basename(file), NULL), EACCES);
> +}
>
> - otfd = TST_RET;
> +static void test_synchronous(void)
> +{
> + strcpy(wbuf, TEST_STR);
> + snprintf(file, PATH_MAX, "%s/synchronous", MNTPOINT);
> + otfd = SAFE_OPEN(file, O_RDWR | O_CREAT, FILE_MODE);
> + SAFE_WRITE(1, otfd, wbuf, strlen(wbuf));
> + SAFE_LSEEK(otfd, 0, SEEK_SET);
> + SAFE_READ(0, otfd, rbuf, sizeof(rbuf));
> + TST_EXP_EQ_STR(rbuf, wbuf);
> }
>
> -static void test_ms_nosuid(void)
> +static void test_remount(void)
> {
> - /* Validate MS_NOSUID flag of mount call */
> + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, MS_REMOUNT, NULL);
> + snprintf(file, PATH_MAX, "%s/remount", MNTPOINT);
> + TST_EXP_FD(otfd = open(file, O_CREAT | O_RDWR, 0700));
> +}
>
> +static void test_nosuid(void)
> +{
> pid_t pid;
> int status;
>
> pid = SAFE_FORK();
> -
> if (!pid) {
> - SAFE_SETREUID(nobody_uid, nobody_gid);
> + SAFE_SETGID(nobody_gid);
> + SAFE_SETREUID(-1, nobody_uid);
> SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> }
>
> - waitpid(pid, &status, 0);
> - if (WIFEXITED(status)) {
> - /* reset the setup_uid */
> - if (status)
> - tst_res(TPASS, "mount(2) passed with flag MS_NOSUID");
> - } else {
> - tst_res(TFAIL, "mount(2) failed with flag MS_NOSUID");
> - }
> -}
> -
> -static void test_ms_nodev(void)
> -{
> - /* Validate MS_NODEV flag of mount call */
> -
> - snprintf(file, PATH_MAX, "%s/mynod_%d", MNTPOINT, getpid());
> - if (SAFE_MKNOD(file, S_IFBLK | 0777, 0) == 0) {
> - TST_EXP_FAIL2(open(file, O_RDWR, 0700),
> - EACCES, "mount(2) passed with flag MS_NODEV: "
> - "open fail with EACCES as expected");
> - otfd = TST_RET;
> - }
> - SAFE_UNLINK(file);
> -}
> + SAFE_WAITPID(pid, &status, 0);
>
> -static void test_noexec(void)
> -{
> - /* Validate MS_NOEXEC flag of mount call */
> - int ret;
> -
> - snprintf(file, PATH_MAX, "%s/tmp1", MNTPOINT);
> - TST_EXP_FD_SILENT(open(file, O_CREAT | O_RDWR, 0700),
> - "opening %s failed", file);
> - otfd = TST_RET;
> - if (otfd >= 0) {
> - SAFE_CLOSE(otfd);
> - ret = execlp(file, basename(file), NULL);
> - if ((ret == -1) && (errno == EACCES)) {
> - tst_res(TPASS, "mount(2) passed with flag MS_NOEXEC");
> + if (WIFEXITED(status)) {
> + switch (WEXITSTATUS(status)) {
> + case EXIT_FAILURE:
> + tst_res(TFAIL, "%s failed", TESTBIN);
> return;
> - }
> - }
> - tst_brk(TFAIL | TERRNO, "mount(2) failed with flag MS_NOEXEC");
> -}
> -
> -static void test_ms_synchronous(void)
> -{
> - /*
> - * Validate MS_SYNCHRONOUS flag of mount call.
> - * Copy some data into data buffer.
> - */
> -
> - strcpy(write_buffer, "abcdefghijklmnopqrstuvwxyz");
> -
> - /* Creat a temporary file under above directory */
> - snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TEMP_FILE);
> - TST_EXP_FD_SILENT(open(file, O_RDWR | O_CREAT, FILE_MODE),
> - "open(%s, O_RDWR|O_CREAT, %#o) failed",
> - file, FILE_MODE);
> - otfd = TST_RET;
> -
> - /* Write the buffer data into file */
> - SAFE_WRITE(1, otfd, write_buffer, strlen(write_buffer));
> -
> - /* Set the file ptr to b'nning of file */
> - SAFE_LSEEK(otfd, 0, SEEK_SET);
> -
> - /* Read the contents of file */
> - if (SAFE_READ(0, otfd, read_buffer, sizeof(read_buffer)) > 0) {
> - if (strcmp(read_buffer, write_buffer)) {
> - tst_brk(TFAIL, "Data read from %s and written "
> - "mismatch", file);
> - } else {
> - SAFE_CLOSE(otfd);
> - tst_res(TPASS, "mount(2) passed with flag MS_SYNCHRONOUS");
> + case EXIT_SUCCESS:
> + tst_res(TPASS, "%s passed", TESTBIN);
> return;
> + default:
> + case TBROK:
> + break;
> }
> - } else {
> - tst_brk(TFAIL | TERRNO, "read() Fails on %s", file);
> }
> -}
>
> -static void test_ms_remount(void)
> -{
> - /* Validate MS_REMOUNT flag of mount call */
> -
> - TEST(mount(tst_device->dev, MNTPOINT, tst_device->fs_type, MS_REMOUNT, NULL));
> - if (TST_RET != 0) {
> - tst_brk(TFAIL | TTERRNO, "mount(2) failed to remount");
> - } else {
> - snprintf(file, PATH_MAX, "%s/tmp2", MNTPOINT);
> - TEST(open(file, O_CREAT | O_RDWR, 0700));
> - otfd = TST_RET;
> - if (otfd == -1) {
> - tst_res(TFAIL, "open(%s) on readonly "
> - "filesystem passed", file);
> - } else
> - tst_res(TPASS, "mount(2) passed with flag MS_REMOUNT");
> - }
> + tst_brk(TBROK, "Child %s", tst_strstatus(status));
> }
>
> -static void test_ms_noatime(void)
> +static void test_noatime(void)
> {
> - /* Validate MS_NOATIME flag of mount call */
> time_t atime;
> - struct stat file_stat;
> + struct stat st;
> char readbuf[20];
>
> - snprintf(file, PATH_MAX, "%s/atime", MNTPOINT);
> - TST_EXP_FD_SILENT(open(file, O_CREAT | O_RDWR, 0700));
> - otfd = TST_RET;
> + snprintf(file, PATH_MAX, "%s/noatime", MNTPOINT);
> + TST_EXP_FD_SILENT(otfd = open(file, O_CREAT | O_RDWR, 0700));
>
> - SAFE_WRITE(1, otfd, "TEST_MS_NOATIME", 15);
> + SAFE_WRITE(1, otfd, TEST_STR, strlen(TEST_STR));
> + SAFE_FSTAT(otfd, &st);
> + atime = st.st_atime;
> + sleep(1);
>
> - SAFE_FSTAT(otfd, &file_stat);
> + SAFE_READ(0, otfd, readbuf, sizeof(readbuf));
> + SAFE_FSTAT(otfd, &st);
> + TST_EXP_EQ_LI(st.st_atime, atime);
> +}
>
> - atime = file_stat.st_atime;
> +#define FLAG_DESC(x) .flag = x, .desc = #x
> +static struct tcase {
> + unsigned int flag;
> + char *desc;
> + void (*test)(void);
> +} tcases[] = {
> + {FLAG_DESC(MS_RDONLY), test_rdonly},
> + {FLAG_DESC(MS_NODEV), test_nodev},
> + {FLAG_DESC(MS_NOEXEC), test_noexec},
> + {FLAG_DESC(MS_SYNCHRONOUS), test_synchronous},
> + {FLAG_DESC(MS_RDONLY), test_remount},
> + {FLAG_DESC(MS_NOSUID), test_nosuid},
> + {FLAG_DESC(MS_NOATIME), test_noatime},
> +};
>
> - sleep(1);
> +static void setup(void)
> +{
> + struct stat st;
> + struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
>
> - SAFE_READ(0, otfd, readbuf, sizeof(readbuf));
> + nobody_uid = ltpuser->pw_uid;
> + nobody_gid = ltpuser->pw_gid;
>
> - SAFE_FSTAT(otfd, &file_stat);
> + snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> + TST_RESOURCE_COPY(NULL, TESTBIN, file);
In fact, old test case copy resource file when mount fileystem, but now,
you change this. So in test_nosuid function, you test nosuid behaviour
in tmpdir instead of different filesystems.
Best Regards
Yang Xu
>
> - if (file_stat.st_atime != atime) {
> - tst_res(TFAIL, "access time is updated");
> - return;
> - }
> - tst_res(TPASS, "mount(2) passed with flag MS_NOATIME");
> + SAFE_STAT(file, &st);
> + if (st.st_mode != SUID_MODE)
> + SAFE_CHMOD(file, SUID_MODE);
> }
>
> -static void run(unsigned int n)
> +static void cleanup(void)
> {
> - struct tcase *tc = &tcases[n];
> -
> - TEST(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
> - tc->flags, NULL));
> - if (tc->do_test)
> - tc->do_test();
> -
> if (otfd >= 0)
> SAFE_CLOSE(otfd);
> +
> if (tst_is_mounted(MNTPOINT))
> SAFE_UMOUNT(MNTPOINT);
> }
>
> -static void cleanup(void)
> -{
> - if (otfd > -1)
> - SAFE_CLOSE(otfd);
> -}
>
> -static void setup(void)
> +static void run(unsigned int n)
> {
> - struct stat file_stat = {0};
> - struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
> + struct tcase *tc = &tcases[n];
>
> - nobody_uid = ltpuser->pw_uid;
> - nobody_gid = ltpuser->pw_gid;
> - snprintf(file, PATH_MAX, "%s/mount03_setuid_test", MNTPOINT);
> - SAFE_STAT(MNTPOINT, &file_stat);
> - if (file_stat.st_mode != SUID_MODE &&
> - chmod(MNTPOINT, SUID_MODE) < 0)
> - tst_brk(TBROK, "setuid for setuid_test failed");
> + tst_res(TINFO, "Testing flag %s", tc->desc);
> +
> + TST_EXP_PASS_SILENT(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
> + tc->flag, NULL));
> + if (!TST_PASS)
> + return;
> +
> + if (tc->test)
> + tc->test();
> +
> + cleanup();
> }
>
> static struct tst_test test = {
> @@ -276,7 +187,7 @@ static struct tst_test test = {
> .needs_root = 1,
> .format_device = 1,
> .resource_files = (const char *const[]) {
> - "mount03_setuid_test",
> + TESTBIN,
> NULL,
> },
> .forks_child = 1,
>
More information about the ltp
mailing list