[LTP] [PATCH v3 0/2] mount03: Convert to new API

Petr Vorel pvorel@suse.cz
Thu Aug 11 15:57:29 CEST 2022


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);
+		SAFE_SETGID(nobody_gid);
+		SAFE_SETREUID(-1, nobody_uid);

* add missing TST_RESOURCE_COPY()
@Cyril: is it really needed?

* 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);
 
-	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