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

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

[Petr Vorel]

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,

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 1/2] tst_test_macros.h: Add TST_EXP_EQ_STR

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
new in v3

 include/tst_test_macros.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
index c8f7825c4..8cc959243 100644
--- a/include/tst_test_macros.h
+++ b/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__ */
-- 
2.37.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

From: "chenhx.fnst@fujitsu.com" <chenhx.fnst@fujitsu.com>

Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
[ pvorel: fixes, cleanup, heavily rewritten ]
Co-developed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: v2 from Chen: https://lore.kernel.org/ltp/20220726070206.266-1-chenhx.fnst@fujitsu.com/
v2->v3 changes are described in the cover letter.

 testcases/kernel/syscalls/mount/mount03.c | 495 +++++++---------------
 1 file changed, 154 insertions(+), 341 deletions(-)

diff --git a/testcases/kernel/syscalls/mount/mount03.c b/testcases/kernel/syscalls/mount/mount03.c
index 25f99bbfc..74b018d78 100644
--- a/testcases/kernel/syscalls/mount/mount03.c
+++ b/testcases/kernel/syscalls/mount/mount03.c
@@ -1,389 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) Linux Test Project, 2022
  * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
  */
 
-/*
- * DESCRIPTION
- *	Check for basic mount(2) system call flags.
+/*\
+ * [Description]
  *
- *	Verify that mount(2) syscall passes for each flag setting and validate
- *	the flags
- *	1) MS_RDONLY - mount read-only.
- *	2) MS_NODEV - disallow access to device special files.
- *	3) MS_NOEXEC - disallow program execution.
- *	4) MS_SYNCHRONOUS - writes are synced at once.
- *	5) MS_REMOUNT - alter flags of a mounted FS.
- *	6) MS_NOSUID - ignore suid and sgid bits.
- *	7) MS_NOATIME - do not update access times.
+ * Verify mount(2) for various flags.
  */
 
-#ifndef _GNU_SOURCE
-#define _GNU_SOURCE
-#endif
-
+#include <stdio.h>
+#include <stdlib.h>
 #include <sys/types.h>
-#include <sys/mount.h>
-#include <sys/stat.h>
 #include <sys/wait.h>
-#include <assert.h>
-#include <errno.h>
-#include <fcntl.h>
 #include <pwd.h>
-#include <unistd.h>
-
-#include "test.h"
-#include "safe_macros.h"
-
-static void setup(void);
-static void cleanup(void);
-static int test_rwflag(int, int);
-
-char *TCID = "mount03";
-int TST_TOTAL = 7;
-
-#define TEMP_FILE	"temp_file"
-#define FILE_MODE	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
-#define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
-			 S_IXGRP|S_IROTH|S_IXOTH)
-#define SUID_MODE	(S_ISUID|S_IRUSR|S_IXUSR|S_IXGRP|S_IXOTH)
-
-static const char mntpoint[] = "mntpoint";
-static const char *device;
-static const char *fs_type;
-static int fildes;
-
-static char write_buffer[BUFSIZ];
-static char read_buffer[BUFSIZ];
-static char path_name[PATH_MAX];
+#include "old_resource.h"
+#include "tst_test.h"
+#include "lapi/mount.h"
+
+#define MNTPOINT        "mntpoint"
+#define TESTBIN	"mount03_setuid_test"
+#define TEST_STR "abcdefghijklmnopqrstuvwxyz"
+#define FILE_MODE	0644
+#define SUID_MODE	0511
+
+static int otfd;
+static char wbuf[BUFSIZ];
+static char rbuf[BUFSIZ];
 static char file[PATH_MAX];
+static uid_t nobody_uid;
+static gid_t nobody_gid;
 
-long rwflags[] = {
-	MS_RDONLY,
-	MS_NODEV,
-	MS_NOEXEC,
-	MS_SYNCHRONOUS,
-	MS_RDONLY,
-	MS_NOSUID,
-	MS_NOATIME,
-};
-
-int main(int argc, char *argv[])
+static void test_rdonly(void)
 {
-	int lc, i;
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
+	snprintf(file, PATH_MAX, "%s/rdonly", MNTPOINT);
+	TST_EXP_FAIL(otfd = open(file, O_CREAT | O_RDWR, 0700), EROFS);
+}
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
+static void test_nodev(void)
+{
+	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);
+}
 
-		tst_count = 0;
+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);
+}
 
-		for (i = 0; i < TST_TOTAL; ++i) {
+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);
+}
 
-			TEST(mount(device, mntpoint, fs_type, rwflags[i],
-				   NULL));
+static void test_remount(void)
+{
+	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));
+}
 
-			if (TEST_RETURN != 0) {
-				tst_resm(TFAIL | TTERRNO, "mount(2) failed");
-				continue;
-			}
+static void test_nosuid(void)
+{
+	pid_t pid;
+	int status;
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		SAFE_SETGID(nobody_gid);
+		SAFE_SETREUID(-1, nobody_uid);
+		SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
+	}
 
-			/* Validate the rwflag */
-			if (test_rwflag(i, lc) == 1)
-				tst_resm(TFAIL, "mount(2) failed while"
-					 " validating %ld", rwflags[i]);
-			else
-				tst_resm(TPASS, "mount(2) passed with "
-					 "rwflag = %ld", rwflags[i]);
+	SAFE_WAITPID(pid, &status, 0);
 
-			TEST(tst_umount(mntpoint));
-			if (TEST_RETURN != 0)
-				tst_brkm(TBROK | TTERRNO, cleanup,
-					 "umount(2) failed for %s", mntpoint);
+	if (WIFEXITED(status)) {
+		switch (WEXITSTATUS(status)) {
+		case EXIT_FAILURE:
+			tst_res(TFAIL, "%s failed", TESTBIN);
+			return;
+		case EXIT_SUCCESS:
+			tst_res(TPASS, "%s passed", TESTBIN);
+			return;
+		default:
+		case TBROK:
+			break;
 		}
 	}
 
-	cleanup();
-	tst_exit();
+	tst_brk(TBROK, "Child %s", tst_strstatus(status));
 }
 
-/*
- * test_rwflag(int i, int cnt)
- * Validate the mount system call for rwflags.
- */
-int test_rwflag(int i, int cnt)
+static void test_noatime(void)
 {
-	int ret, fd, pid, status;
-	char nobody_uid[] = "nobody";
 	time_t atime;
-	struct passwd *ltpuser;
-	struct stat file_stat;
+	struct stat st;
 	char readbuf[20];
 
-	switch (i) {
-	case 0:
-		/* Validate MS_RDONLY flag of mount call */
-
-		snprintf(file, PATH_MAX, "%stmp", path_name);
-		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
-		if (fd == -1) {
-			if (errno == EROFS) {
-				return 0;
-			} else {
-				tst_resm(TWARN | TERRNO,
-					 "open didn't fail with EROFS");
-				return 1;
-			}
-		}
-		close(fd);
-		return 1;
-	case 1:
-		/* Validate MS_NODEV flag of mount call */
-
-		snprintf(file, PATH_MAX, "%smynod_%d_%d", path_name, getpid(),
-			 cnt);
-		if (mknod(file, S_IFBLK | 0777, 0) == 0) {
-			fd = open(file, O_RDWR, S_IRWXU);
-			if (fd == -1) {
-				if (errno == EACCES) {
-					return 0;
-				} else {
-					tst_resm(TWARN | TERRNO,
-						 "open didn't fail with EACCES");
-					return 1;
-				}
-			}
-			close(fd);
-		} else {
-			tst_resm(TWARN | TERRNO, "mknod(2) failed to create %s",
-				 file);
-			return 1;
-		}
-		return 1;
-	case 2:
-		/* Validate MS_NOEXEC flag of mount call */
-
-		snprintf(file, PATH_MAX, "%stmp1", path_name);
-		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
-		if (fd == -1) {
-			tst_resm(TWARN | TERRNO, "opening %s failed", file);
-		} else {
-			close(fd);
-			ret = execlp(file, basename(file), NULL);
-			if ((ret == -1) && (errno == EACCES))
-				return 0;
-		}
-		return 1;
-	case 3:
-		/*
-		 * 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", path_name, TEMP_FILE);
-		fildes = open(file, O_RDWR | O_CREAT, FILE_MODE);
-		if (fildes == -1) {
-			tst_resm(TWARN | TERRNO,
-				 "open(%s, O_RDWR|O_CREAT, %#o) failed",
-				 file, FILE_MODE);
-			return 1;
-		}
+	snprintf(file, PATH_MAX, "%s/noatime", MNTPOINT);
+	TST_EXP_FD_SILENT(otfd = open(file, O_CREAT | O_RDWR, 0700));
 
-		/* Write the buffer data into file */
-		if (write(fildes, write_buffer, strlen(write_buffer)) !=
-		    (long)strlen(write_buffer)) {
-			tst_resm(TWARN | TERRNO, "writing to %s failed", file);
-			close(fildes);
-			return 1;
-		}
+	SAFE_WRITE(1, otfd, TEST_STR, strlen(TEST_STR));
+	SAFE_FSTAT(otfd, &st);
+	atime = st.st_atime;
+	sleep(1);
 
-		/* Set the file ptr to b'nning of file */
-		if (lseek(fildes, 0, SEEK_SET) < 0) {
-			tst_resm(TWARN, "lseek() failed on %s, error="
-				 " %d", file, errno);
-			close(fildes);
-			return 1;
-		}
-
-		/* Read the contents of file */
-		if (read(fildes, read_buffer, sizeof(read_buffer)) > 0) {
-			if (strcmp(read_buffer, write_buffer)) {
-				tst_resm(TWARN, "Data read from %s and written "
-					 "mismatch", file);
-				close(fildes);
-				return 1;
-			} else {
-				close(fildes);
-				return 0;
-			}
-		} else {
-			tst_resm(TWARN | TERRNO, "read() Fails on %s", file);
-			close(fildes);
-			return 1;
-		}
-
-	case 4:
-		/* Validate MS_REMOUNT flag of mount call */
-
-		TEST(mount(device, mntpoint, fs_type, MS_REMOUNT, NULL));
-		if (TEST_RETURN != 0) {
-			tst_resm(TWARN | TTERRNO, "mount(2) failed to remount");
-			return 1;
-		} else {
-			snprintf(file, PATH_MAX, "%stmp2", path_name);
-			fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
-			if (fd == -1) {
-				tst_resm(TWARN, "open(%s) on readonly "
-					 "filesystem passed", file);
-				return 1;
-			} else {
-				close(fd);
-				return 0;
-			}
-		}
-	case 5:
-		/* Validate MS_NOSUID flag of mount call */
-
-		snprintf(file, PATH_MAX, "%smount03_setuid_test", path_name);
-
-		pid = fork();
-		switch (pid) {
-		case -1:
-			tst_resm(TBROK | TERRNO, "fork failed");
-			return 1;
-		case 0:
-			ltpuser = getpwnam(nobody_uid);
-			if (setreuid(ltpuser->pw_uid, ltpuser->pw_uid) == -1)
-				tst_resm(TWARN | TERRNO,
-					 "seteuid() failed to change euid to %d",
-					 ltpuser->pw_uid);
-
-			execlp(file, basename(file), NULL);
-			exit(1);
-		default:
-			waitpid(pid, &status, 0);
-			if (WIFEXITED(status)) {
-				/* reset the setup_uid */
-				if (status)
-					return 0;
-			}
-			return 1;
-		}
-	case 6:
-		/* Validate MS_NOATIME flag of mount call */
-
-		snprintf(file, PATH_MAX, "%satime", path_name);
-		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
-		if (fd == -1) {
-			tst_resm(TWARN | TERRNO, "opening %s failed", file);
-			return 1;
-		}
-
-		if (write(fd, "TEST_MS_NOATIME", 15) != 15) {
-			tst_resm(TWARN | TERRNO, "write %s failed", file);
-			close(fd);
-			return 1;
-		}
-
-		if (fstat(fd, &file_stat) == -1) {
-			tst_resm(TWARN | TERRNO, "stat %s failed #1", file);
-			close(fd);
-			return 1;
-		}
-
-		atime = file_stat.st_atime;
-
-		sleep(1);
-
-		if (read(fd, readbuf, sizeof(readbuf)) == -1) {
-			tst_resm(TWARN | TERRNO, "read %s failed", file);
-			close(fd);
-			return 1;
-		}
-
-		if (fstat(fd, &file_stat) == -1) {
-			tst_resm(TWARN | TERRNO, "stat %s failed #2", file);
-			close(fd);
-			return 1;
-		}
-		close(fd);
-
-		if (file_stat.st_atime != atime) {
-			tst_resm(TWARN, "access time is updated");
-			return 1;
-		}
-		return 0;
-	}
-	return 0;
+	SAFE_READ(0, otfd, readbuf, sizeof(readbuf));
+	SAFE_FSTAT(otfd, &st);
+	TST_EXP_EQ_LI(st.st_atime, 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},
+};
+
 static void setup(void)
 {
-	char path[PATH_MAX];
-	struct stat file_stat;
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
+	struct stat st;
+	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
 
-	tst_require_root();
+	nobody_uid = ltpuser->pw_uid;
+	nobody_gid = ltpuser->pw_gid;
 
-	tst_tmpdir();
+	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
+	TST_RESOURCE_COPY(NULL, TESTBIN, file);
 
-	fs_type = tst_dev_fs_type();
-	device = tst_acquire_device(cleanup);
-
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-
-	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
+	SAFE_STAT(file, &st);
+	if (st.st_mode != SUID_MODE)
+	    SAFE_CHMOD(file, SUID_MODE);
+}
 
-	SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
+static void cleanup(void)
+{
+	if (otfd >= 0)
+		SAFE_CLOSE(otfd);
 
-	if (getcwd(path_name, sizeof(path_name)) == NULL)
-		tst_brkm(TBROK, cleanup, "getcwd failed");
+	if (tst_is_mounted(MNTPOINT))
+		SAFE_UMOUNT(MNTPOINT);
+}
 
-	if (chmod(path_name, DIR_MODE) != 0)
-		tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
-			 path_name, DIR_MODE);
 
-	strncpy(path, path_name, PATH_MAX);
-	snprintf(path_name, PATH_MAX, "%s/%s/", path, mntpoint);
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
 
-	SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
-	TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
+	tst_res(TINFO, "Testing flag %s", tc->desc);
 
-	snprintf(file, PATH_MAX, "%smount03_setuid_test", path_name);
-	SAFE_STAT(cleanup, file, &file_stat);
+	TST_EXP_PASS_SILENT(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
+		   tc->flag, NULL));
+	if (!TST_PASS)
+		return;
 
-	if (file_stat.st_mode != SUID_MODE &&
-	    chmod(file, SUID_MODE) < 0)
-		tst_brkm(TBROK, cleanup,
-			 "setuid for setuid_test failed");
-	SAFE_UMOUNT(cleanup, mntpoint);
+	if (tc->test)
+		tc->test();
 
-	TEST_PAUSE;
+	cleanup();
 }
 
-static void cleanup(void)
-{
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.format_device = 1,
+	.resource_files = (const char *const[]) {
+		TESTBIN,
+		NULL,
+	},
+	.forks_child = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *const []){
+		"exfat",
+		"vfat",
+		"ntfs",
+		NULL
+	},
+};
-- 
2.37.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] tst_test_macros.h: Add TST_EXP_EQ_STR

[xuyang2018.jy]

Hi Petr

Looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Best Regards
Yang Xu
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> new in v3
> 
>   include/tst_test_macros.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
> index c8f7825c4..8cc959243 100644
> --- a/include/tst_test_macros.h
> +++ b/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__ */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

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,
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

> 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?

Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
causes mount03_setuid_test to fail (exit 1).
The same code is in creat08.c, creat09.c, open10.c.
Did I answer your question?

> > * 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.

Ah, thanks!
SAFE_CP(TESTBIN, file);

...
> > +#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.

old code in setup:
    fs_type = tst_dev_fs_type();
    device = tst_acquire_device(cleanup);

    if (!device)
        tst_brkm(TCONF, cleanup, "Failed to obtain block device");

    tst_mkfs(cleanup, device, fs_type, NULL, NULL);

    SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);

    SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
    TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);

new code:
    snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
    SAFE_CP(TESTBIN, file);

Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?

But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
(or at least different from the old code).

Kind regards,
Petr

> Best Regards
> Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
>> 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?
> 
> Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
> causes mount03_setuid_test to fail (exit 1).
> The same code is in creat08.c, creat09.c, open10.c.
> Did I answer your question?

Yes.
> 
>>> * 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.
> 
> Ah, thanks!
> SAFE_CP(TESTBIN, file);
> 
> ...
>>> +#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.
> 
> old code in setup:
>      fs_type = tst_dev_fs_type();
>      device = tst_acquire_device(cleanup);
> 
>      if (!device)
>          tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> 
>      tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> 
>      SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
> 
>      SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
>      TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
> 
> new code:
>      snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>      SAFE_CP(TESTBIN, file);
> 
> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?
> 
> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
> (or at least different from the old code).

Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just 
create mntpoint instead mount dev to a moutpoint).

So do you will fix this？

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
> 
>> Best Regards
>> Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

...
> >>> -	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.

> > old code in setup:
> >      fs_type = tst_dev_fs_type();
> >      device = tst_acquire_device(cleanup);

> >      if (!device)
> >          tst_brkm(TCONF, cleanup, "Failed to obtain block device");

> >      tst_mkfs(cleanup, device, fs_type, NULL, NULL);

> >      SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);

> >      SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
> >      TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);

> > new code:
> >      snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> >      SAFE_CP(TESTBIN, file);

> > Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
> > struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?

> > But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
> > (or at least different from the old code).

> Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just 
> create mntpoint instead mount dev to a moutpoint).

> So do you will fix this？

Yes, see the diff below. I'm waiting little longer if anybody else has some
comments before merging it.

Thanks for your review.

> Best Regards
> Yang Xu

diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
index 74b018d78..9c58783d7 100644
--- testcases/kernel/syscalls/mount/mount03.c
+++ testcases/kernel/syscalls/mount/mount03.c
@@ -15,7 +15,6 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <pwd.h>
-#include "old_resource.h"
 #include "tst_test.h"
 #include "lapi/mount.h"
 
@@ -145,7 +144,7 @@ static void setup(void)
 	nobody_gid = ltpuser->pw_gid;
 
 	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
-	TST_RESOURCE_COPY(NULL, TESTBIN, file);
+	SAFE_CP(TESTBIN, file);
 
 	SAFE_STAT(file, &st);
 	if (st.st_mode != SUID_MODE)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
> ...
>>>>> -	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.
> 
>>> old code in setup:
>>>       fs_type = tst_dev_fs_type();
>>>       device = tst_acquire_device(cleanup);
> 
>>>       if (!device)
>>>           tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> 
>>>       tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> 
>>>       SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
> 
>>>       SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
>>>       TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
> 
>>> new code:
>>>       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>>>       SAFE_CP(TESTBIN, file);
> 
>>> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
>>> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?
> 
>>> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
>>> (or at least different from the old code).
> 
>> Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just
>> create mntpoint instead mount dev to a moutpoint).
> 
>> So do you will fix this？
> 
> Yes, see the diff below. I'm waiting little longer if anybody else has some
> comments before merging it.
> 
> Thanks for your review.
> 
>> Best Regards
>> Yang Xu
> 
> diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> index 74b018d78..9c58783d7 100644
> --- testcases/kernel/syscalls/mount/mount03.c
> +++ testcases/kernel/syscalls/mount/mount03.c
> @@ -15,7 +15,6 @@
>   #include <sys/types.h>
>   #include <sys/wait.h>
>   #include <pwd.h>
> -#include "old_resource.h"
>   #include "tst_test.h"
>   #include "lapi/mount.h"
>   
> @@ -145,7 +144,7 @@ static void setup(void)
>   	nobody_gid = ltpuser->pw_gid;
>   
>   	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> +	SAFE_CP(TESTBIN, file);

I still think we should test nosuid behaviour on different filesystem 
like other test function because we have expand it to all filesystems.

Also include tmpfs, so SAFE_CP should be in test_nosuid function 
otherwise may hit ENOENT problem.

different code as below:

[root@localhost mount]# git diff .
diff --git a/testcases/kernel/syscalls/mount/mount03.c 
b/testcases/kernel/syscalls/mount/mount03.c
index 74b018d78..b0582c76b 100644
--- a/testcases/kernel/syscalls/mount/mount03.c
+++ b/testcases/kernel/syscalls/mount/mount03.c
@@ -21,6 +21,7 @@

  #define MNTPOINT        "mntpoint"
  #define TESTBIN        "mount03_setuid_test"
+#define BIN_PATH           MNTPOINT"/"TESTBIN
  #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
  #define FILE_MODE      0644
  #define SUID_MODE      0511
@@ -75,12 +76,19 @@ static void test_nosuid(void)
  {
         pid_t pid;
         int status;
+       struct stat st;
+
+       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
+       SAFE_CP(TESTBIN, file);
+       SAFE_STAT(file, &st);
+       if (st.st_mode != SUID_MODE)
+               SAFE_CHMOD(file, SUID_MODE);

         pid = SAFE_FORK();
         if (!pid) {
                 SAFE_SETGID(nobody_gid);
                 SAFE_SETREUID(-1, nobody_uid);
-               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
+               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
         }

         SAFE_WAITPID(pid, &status, 0);
@@ -138,18 +146,10 @@ static struct tcase {

  static void setup(void)
  {
-       struct stat st;
         struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

         nobody_uid = ltpuser->pw_uid;
         nobody_gid = ltpuser->pw_gid;
-
-       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
-       TST_RESOURCE_COPY(NULL, TESTBIN, file);
-
-       SAFE_STAT(file, &st);
-       if (st.st_mode != SUID_MODE)
-           SAFE_CHMOD(file, SUID_MODE);
  }

  static void cleanup(void)
[root@localhost mount]#


Best Regards
Yang Xu

>   
>   	SAFE_STAT(file, &st);
>   	if (st.st_mode != SUID_MODE)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

...
> > diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> > index 74b018d78..9c58783d7 100644
> > --- testcases/kernel/syscalls/mount/mount03.c
> > +++ testcases/kernel/syscalls/mount/mount03.c
> > @@ -15,7 +15,6 @@
> >   #include <sys/types.h>
> >   #include <sys/wait.h>
> >   #include <pwd.h>
> > -#include "old_resource.h"
> >   #include "tst_test.h"
> >   #include "lapi/mount.h"

> > @@ -145,7 +144,7 @@ static void setup(void)
> >   	nobody_gid = ltpuser->pw_gid;

> >   	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> > +	SAFE_CP(TESTBIN, file);

> I still think we should test nosuid behaviour on different filesystem 
> like other test function because we have expand it to all filesystems.

> Also include tmpfs, so SAFE_CP should be in test_nosuid function 
> otherwise may hit ENOENT problem.

Ah thx, good idea. I guess the point of the setup was to run copy only once, but
your points are obviously valid.

I didn't notice it before because I overlooked SAFE_EXECLP() in test_nosuid() it
had parameter TESTBIN, thus not being run from mountpoint.

nit: I suggest to move to SAFE_EXECL() as it expect path, not filename as it's
not using PATH. Similarly we could change execlp() to execl() in test_noexec(),
but I'd prefer to keep execlp(), so that we test two different libc wrappers.

> different code as below:

> [root@localhost mount]# git diff .
> diff --git a/testcases/kernel/syscalls/mount/mount03.c 
> b/testcases/kernel/syscalls/mount/mount03.c
> index 74b018d78..b0582c76b 100644
> --- a/testcases/kernel/syscalls/mount/mount03.c
> +++ b/testcases/kernel/syscalls/mount/mount03.c
> @@ -21,6 +21,7 @@

>   #define MNTPOINT        "mntpoint"
>   #define TESTBIN        "mount03_setuid_test"
> +#define BIN_PATH           MNTPOINT"/"TESTBIN
+1 for avoid the need of snprintf when there are 2 constants.
NOTE: we can separate 3 items with spaces:
#define BIN_PATH	MNTPOINT "/" TESTBIN
But I'd rename it to TESTBIN_PATH.
Or maybe even better to use just "TEST":
#define TEST        "mount03_setuid_test"
#define TEST_PATH	MNTPOINT "/" TEST

>   #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
>   #define FILE_MODE      0644
>   #define SUID_MODE      0511
> @@ -75,12 +76,19 @@ static void test_nosuid(void)
>   {
>          pid_t pid;
>          int status;
> +       struct stat st;
> +
> +       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
this is not needed when we have BIN_PATH
> +       SAFE_CP(TESTBIN, file);
SAFE_CP(TESTBIN, BIN_PATH);
> +       SAFE_STAT(file, &st);
> +       if (st.st_mode != SUID_MODE)
> +               SAFE_CHMOD(file, SUID_MODE);
	SAFE_CHMOD(BIN_PATH, SUID_MODE);

>          pid = SAFE_FORK();
>          if (!pid) {
>                  SAFE_SETGID(nobody_gid);
>                  SAFE_SETREUID(-1, nobody_uid);
> -               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> +               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
>          }

>          SAFE_WAITPID(pid, &status, 0);
> @@ -138,18 +146,10 @@ static struct tcase {

>   static void setup(void)
>   {
> -       struct stat st;
>          struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

>          nobody_uid = ltpuser->pw_uid;
>          nobody_gid = ltpuser->pw_gid;
> -
> -       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> -       TST_RESOURCE_COPY(NULL, TESTBIN, file);
> -
> -       SAFE_STAT(file, &st);
> -       if (st.st_mode != SUID_MODE)
> -           SAFE_CHMOD(file, SUID_MODE);
>   }

>   static void cleanup(void)

Final diff is below, but for readability it's temporarily also on my fork:
https://github.com/pevik/ltp/blob/57ba1ba47987a201c39764b4259a15aa39db9d2e/testcases/kernel/syscalls/mount/mount03.c

Kind regards,
Petr

> Best Regards
> Yang Xu

diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
index 9c58783d7..eef2a65c6 100644
--- testcases/kernel/syscalls/mount/mount03.c
+++ testcases/kernel/syscalls/mount/mount03.c
@@ -18,8 +18,9 @@
 #include "tst_test.h"
 #include "lapi/mount.h"
 
-#define MNTPOINT        "mntpoint"
+#define MNTPOINT	"mntpoint"
 #define TESTBIN	"mount03_setuid_test"
+#define BIN_PATH	MNTPOINT "/" TESTBIN
 #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
 #define FILE_MODE	0644
 #define SUID_MODE	0511
@@ -74,12 +75,18 @@ static void test_nosuid(void)
 {
 	pid_t pid;
 	int status;
+	struct stat st;
+
+	SAFE_CP(TESTBIN, BIN_PATH);
+	SAFE_STAT(BIN_PATH, &st);
+	if (st.st_mode != SUID_MODE)
+		SAFE_CHMOD(BIN_PATH, SUID_MODE);
 
 	pid = SAFE_FORK();
 	if (!pid) {
 		SAFE_SETGID(nobody_gid);
 		SAFE_SETREUID(-1, nobody_uid);
-		SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
+		SAFE_EXECL(BIN_PATH, TESTBIN, NULL);
 	}
 
 	SAFE_WAITPID(pid, &status, 0);
@@ -137,18 +144,10 @@ static struct tcase {
 
 static void setup(void)
 {
-	struct stat st;
 	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
 
 	nobody_uid = ltpuser->pw_uid;
 	nobody_gid = ltpuser->pw_gid;
-
-	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
-	SAFE_CP(TESTBIN, file);
-
-	SAFE_STAT(file, &st);
-	if (st.st_mode != SUID_MODE)
-	    SAFE_CHMOD(file, SUID_MODE);
 }
 
 static void cleanup(void)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
> ...
>>> diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
>>> index 74b018d78..9c58783d7 100644
>>> --- testcases/kernel/syscalls/mount/mount03.c
>>> +++ testcases/kernel/syscalls/mount/mount03.c
>>> @@ -15,7 +15,6 @@
>>>    #include <sys/types.h>
>>>    #include <sys/wait.h>
>>>    #include <pwd.h>
>>> -#include "old_resource.h"
>>>    #include "tst_test.h"
>>>    #include "lapi/mount.h"
> 
>>> @@ -145,7 +144,7 @@ static void setup(void)
>>>    	nobody_gid = ltpuser->pw_gid;
> 
>>>    	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>>> -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
>>> +	SAFE_CP(TESTBIN, file);
> 
>> I still think we should test nosuid behaviour on different filesystem
>> like other test function because we have expand it to all filesystems.
> 
>> Also include tmpfs, so SAFE_CP should be in test_nosuid function
>> otherwise may hit ENOENT problem.
> 
> Ah thx, good idea. I guess the point of the setup was to run copy only once, but
> your points are obviously valid.
> 
> I didn't notice it before because I overlooked SAFE_EXECLP() in test_nosuid() it
> had parameter TESTBIN, thus not being run from mountpoint.
> 
> nit: I suggest to move to SAFE_EXECL() as it expect path, not filename as it's
> not using PATH. Similarly we could change execlp() to execl() in test_noexec(),
> but I'd prefer to keep execlp(), so that we test two different libc wrappers.

Sound reasonable.
> 
>> different code as below:
> 
>> [root@localhost mount]# git diff .
>> diff --git a/testcases/kernel/syscalls/mount/mount03.c
>> b/testcases/kernel/syscalls/mount/mount03.c
>> index 74b018d78..b0582c76b 100644
>> --- a/testcases/kernel/syscalls/mount/mount03.c
>> +++ b/testcases/kernel/syscalls/mount/mount03.c
>> @@ -21,6 +21,7 @@
> 
>>    #define MNTPOINT        "mntpoint"
>>    #define TESTBIN        "mount03_setuid_test"
>> +#define BIN_PATH           MNTPOINT"/"TESTBIN
> +1 for avoid the need of snprintf when there are 2 constants.
> NOTE: we can separate 3 items with spaces:
> #define BIN_PATH	MNTPOINT "/" TESTBIN
> But I'd rename it to TESTBIN_PATH.
> Or maybe even better to use just "TEST":
> #define TEST        "mount03_setuid_test"
> #define TEST_PATH	MNTPOINT "/" TEST

Looks well.
> 
>>    #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
>>    #define FILE_MODE      0644
>>    #define SUID_MODE      0511
>> @@ -75,12 +76,19 @@ static void test_nosuid(void)
>>    {
>>           pid_t pid;
>>           int status;
>> +       struct stat st;
>> +
>> +       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> this is not needed when we have BIN_PATH
Yes.
>> +       SAFE_CP(TESTBIN, file);
> SAFE_CP(TESTBIN, BIN_PATH);
>> +       SAFE_STAT(file, &st);
>> +       if (st.st_mode != SUID_MODE)
>> +               SAFE_CHMOD(file, SUID_MODE);
> 	SAFE_CHMOD(BIN_PATH, SUID_MODE);
> 
>>           pid = SAFE_FORK();
>>           if (!pid) {
>>                   SAFE_SETGID(nobody_gid);
>>                   SAFE_SETREUID(-1, nobody_uid);
>> -               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
>> +               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
>>           }
> 
>>           SAFE_WAITPID(pid, &status, 0);
>> @@ -138,18 +146,10 @@ static struct tcase {
> 
>>    static void setup(void)
>>    {
>> -       struct stat st;
>>           struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
> 
>>           nobody_uid = ltpuser->pw_uid;
>>           nobody_gid = ltpuser->pw_gid;
>> -
>> -       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>> -       TST_RESOURCE_COPY(NULL, TESTBIN, file);
>> -
>> -       SAFE_STAT(file, &st);
>> -       if (st.st_mode != SUID_MODE)
>> -           SAFE_CHMOD(file, SUID_MODE);
>>    }
> 
>>    static void cleanup(void)
> 
> Final diff is below, but for readability it's temporarily also on my fork:
> https://github.com/pevik/ltp/blob/57ba1ba47987a201c39764b4259a15aa39db9d2e/testcases/kernel/syscalls/mount/mount03.c

OK.
> 
> Kind regards,
> Petr
> 
>> Best Regards
>> Yang Xu
> 
> diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> index 9c58783d7..eef2a65c6 100644
> --- testcases/kernel/syscalls/mount/mount03.c
> +++ testcases/kernel/syscalls/mount/mount03.c
> @@ -18,8 +18,9 @@
>   #include "tst_test.h"
>   #include "lapi/mount.h"
>   
> -#define MNTPOINT        "mntpoint"
> +#define MNTPOINT	"mntpoint"
>   #define TESTBIN	"mount03_setuid_test"
> +#define BIN_PATH	MNTPOINT "/" TESTBIN
>   #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
>   #define FILE_MODE	0644
>   #define SUID_MODE	0511

Here SUID_MODE miss S_ISUID, that is why this case also pass if we 
excute a program on filesystem that not mounted with nosuid option.

Also, it seems make check code has problem when detecting S_ISUID

Symbolic permissions 'S_IRUSR|S_IXUSR|S_IXGRP|S_IXOTH' are not 
preferred. Consider using octal permissions '0511'.

Since we have converted mount03 into new api, mount03_setuid_test.c 
should also be converted into new spdx.

Best Regards
Yang Xu

> @@ -74,12 +75,18 @@ static void test_nosuid(void)
>   {
>   	pid_t pid;
>   	int status;
> +	struct stat st;
> +
> +	SAFE_CP(TESTBIN, BIN_PATH);
> +	SAFE_STAT(BIN_PATH, &st);
> +	if (st.st_mode != SUID_MODE)
> +		SAFE_CHMOD(BIN_PATH, SUID_MODE);
>   
>   	pid = SAFE_FORK();
>   	if (!pid) {
>   		SAFE_SETGID(nobody_gid);
>   		SAFE_SETREUID(-1, nobody_uid);
> -		SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> +		SAFE_EXECL(BIN_PATH, TESTBIN, NULL);
>   	}
>   
>   	SAFE_WAITPID(pid, &status, 0);
> @@ -137,18 +144,10 @@ static struct tcase {
>   
>   static void setup(void)
>   {
> -	struct stat st;
>   	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
>   
>   	nobody_uid = ltpuser->pw_uid;
>   	nobody_gid = ltpuser->pw_gid;
> -
> -	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> -	SAFE_CP(TESTBIN, file);
> -
> -	SAFE_STAT(file, &st);
> -	if (st.st_mode != SUID_MODE)
> -	    SAFE_CHMOD(file, SUID_MODE);
>   }
>   
>   static void cleanup(void)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr> Hi Xu,
> 
>> 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?
> 
> Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
> causes mount03_setuid_test to fail (exit 1).
> The same code is in creat08.c, creat09.c, open10.c.
> Did I answer your question?

I look mount03_setuid_test code today, nosuid mount option should
expect setuid failed when using a non-privileged user even this program 
has set-user-id bit.

Old api also think PASS when mount03_setuid_test exit 1

So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
and then use code as below:

	if (WIFEXITED(status)) {
		switch (WEXITSTATUS(status)) {
		case EXIT_FAILURE:
			tst_res(TPASS, "%s passed", TESTBIN);
			return;
		case EXIT_SUCCESS:
			tst_res(TFAIL, "%s failed", TESTBIN);
			return;
		default:
		case TBROK:
			break;
		}
	}

Best Regards
Yang Xu
> 
>>> * 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.
> 
> Ah, thanks!
> SAFE_CP(TESTBIN, file);
> 
> ...
>>> +#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.
> 
> old code in setup:
>      fs_type = tst_dev_fs_type();
>      device = tst_acquire_device(cleanup);
> 
>      if (!device)
>          tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> 
>      tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> 
>      SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
> 
>      SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
>      TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
> 
> new code:
>      snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
>      SAFE_CP(TESTBIN, file);
> 
> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?
> 
> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
> (or at least different from the old code).
> 
> Kind regards,
> Petr
> 
>> Best Regards
>> Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

> >> Why here is nobody_gid?

> >>> +		SAFE_SETGID(nobody_gid);
> >>> +		SAFE_SETREUID(-1, nobody_uid);

> >> What problem do you meet?

> > Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
> > causes mount03_setuid_test to fail (exit 1).
> > The same code is in creat08.c, creat09.c, open10.c.
> > Did I answer your question?

> I look mount03_setuid_test code today, nosuid mount option should
> expect setuid failed when using a non-privileged user even this program 
> has set-user-id bit.

> Old api also think PASS when mount03_setuid_test exit 1

Ah, thanks for catching my error!

> So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
> and then use code as below:

> 	if (WIFEXITED(status)) {
> 		switch (WEXITSTATUS(status)) {
> 		case EXIT_FAILURE:
> 			tst_res(TPASS, "%s passed", TESTBIN);
> 			return;
> 		case EXIT_SUCCESS:
> 			tst_res(TFAIL, "%s failed", TESTBIN);
> 			return;
> 		default:
> 		case TBROK:
> 			break;
> 		}
I guess we can drop the default and TBROK part, right?
It's caught later by tst_brk(TBROK, ...)
> 	}

https://github.com/pevik/ltp/blob/22652d668a5ccbf3c7aa835c2dab6d0eb6058ba2/testcases/kernel/syscalls/mount/mount03.c#L74-L105

static void test_nosuid(void)
{
	pid_t pid;
	int status;
	struct stat st;

	SAFE_CP(TESTBIN, BIN_PATH);
	SAFE_STAT(BIN_PATH, &st);
	if (st.st_mode != SUID_MODE)
		SAFE_CHMOD(BIN_PATH, SUID_MODE);

	pid = SAFE_FORK();
	if (!pid) {
		SAFE_SETREUID(nobody_uid, nobody_uid);
		SAFE_EXECL(BIN_PATH, TESTBIN, NULL);
	}

	SAFE_WAITPID(pid, &status, 0);

	if (WIFEXITED(status)) {
		switch (WEXITSTATUS(status)) {
		case EXIT_FAILURE:
			tst_res(TPASS, "%s passed", TESTBIN);
			return;
		case EXIT_SUCCESS:
			tst_res(TFAIL, "%s failed", TESTBIN);
			return;
		}
	}

	tst_brk(TBROK, "Child %s", tst_strstatus(status));
}

Kind regards,
Petr

> Best Regards
> Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
>>>> Why here is nobody_gid?
> 
>>>>> +		SAFE_SETGID(nobody_gid);
>>>>> +		SAFE_SETREUID(-1, nobody_uid);
> 
>>>> What problem do you meet?
> 
>>> Using original code SAFE_SETREUID(nobody_uid, nobody_gid);
>>> causes mount03_setuid_test to fail (exit 1).
>>> The same code is in creat08.c, creat09.c, open10.c.
>>> Did I answer your question?
> 
>> I look mount03_setuid_test code today, nosuid mount option should
>> expect setuid failed when using a non-privileged user even this program
>> has set-user-id bit.
> 
>> Old api also think PASS when mount03_setuid_test exit 1
> 
> Ah, thanks for catching my error!
> 
>> So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
>> and then use code as below:
> 
>> 	if (WIFEXITED(status)) {
>> 		switch (WEXITSTATUS(status)) {
>> 		case EXIT_FAILURE:
>> 			tst_res(TPASS, "%s passed", TESTBIN);
>> 			return;
>> 		case EXIT_SUCCESS:
>> 			tst_res(TFAIL, "%s failed", TESTBIN);
>> 			return;
>> 		default:
>> 		case TBROK:
>> 			break;
>> 		}
> I guess we can drop the default and TBROK part, right?
> It's caught later by tst_brk(TBROK, ...)
>> 	}

Yes, I am fine with drop this.

Best Regards
Yang Xu
> 
> https://github.com/pevik/ltp/blob/22652d668a5ccbf3c7aa835c2dab6d0eb6058ba2/testcases/kernel/syscalls/mount/mount03.c#L74-L105
> 
> static void test_nosuid(void)
> {
> 	pid_t pid;
> 	int status;
> 	struct stat st;
> 
> 	SAFE_CP(TESTBIN, BIN_PATH);
> 	SAFE_STAT(BIN_PATH, &st);
> 	if (st.st_mode != SUID_MODE)
> 		SAFE_CHMOD(BIN_PATH, SUID_MODE);
> 
> 	pid = SAFE_FORK();
> 	if (!pid) {
> 		SAFE_SETREUID(nobody_uid, nobody_uid);
> 		SAFE_EXECL(BIN_PATH, TESTBIN, NULL);
> 	}
> 
> 	SAFE_WAITPID(pid, &status, 0);
> 
> 	if (WIFEXITED(status)) {
> 		switch (WEXITSTATUS(status)) {
> 		case EXIT_FAILURE:
> 			tst_res(TPASS, "%s passed", TESTBIN);
> 			return;
> 		case EXIT_SUCCESS:
> 			tst_res(TFAIL, "%s failed", TESTBIN);
> 			return;
> 		}
> 	}
> 
> 	tst_brk(TBROK, "Child %s", tst_strstatus(status));
> }
> 
> Kind regards,
> Petr
> 
>> Best Regards
>> Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Cyril Hrubis]

Hi!
> So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
> and then use code as below:
> 
> 	if (WIFEXITED(status)) {
> 		switch (WEXITSTATUS(status)) {
> 		case EXIT_FAILURE:
> 			tst_res(TPASS, "%s passed", TESTBIN);
> 			return;
> 		case EXIT_SUCCESS:
> 			tst_res(TFAIL, "%s failed", TESTBIN);
> 			return;
> 		default:
> 		case TBROK:
> 			break;
> 		}
> 	}

Can we please stop propagating test success/failure via the exit value?

The problem have been solved years ago by the new test library, the
child just need to call tst_reinit() and tst_res() will work directly
from the child as well. See execlp01.c for example how that works.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

> Hi!
> > So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
> > and then use code as below:

> > 	if (WIFEXITED(status)) {
> > 		switch (WEXITSTATUS(status)) {
> > 		case EXIT_FAILURE:
> > 			tst_res(TPASS, "%s passed", TESTBIN);
> > 			return;
> > 		case EXIT_SUCCESS:
> > 			tst_res(TFAIL, "%s failed", TESTBIN);
> > 			return;
> > 		default:
> > 		case TBROK:
> > 			break;
> > 		}
> > 	}

> Can we please stop propagating test success/failure via the exit value?

> The problem have been solved years ago by the new test library, the
> child just need to call tst_reinit() and tst_res() will work directly
> from the child as well. See execlp01.c for example how that works.

Ah, thanks! I should have read C API docs once more (it's in "1.8 Doing the test
in the child process"). At this point there are so many changes => I'll post v4
and cleanup the child as well.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Cyril Hrubis]

Hi!
> --- a/testcases/kernel/syscalls/mount/mount03.c
> +++ b/testcases/kernel/syscalls/mount/mount03.c
> @@ -1,389 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (c) Linux Test Project, 2022
>   * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.

This is GPL-2.0 not GPL-2.0-or-later

> -/*
> - * DESCRIPTION
> - *	Check for basic mount(2) system call flags.
> +/*\
> + * [Description]
>   *
> - *	Verify that mount(2) syscall passes for each flag setting and validate
> - *	the flags
> - *	1) MS_RDONLY - mount read-only.
> - *	2) MS_NODEV - disallow access to device special files.
> - *	3) MS_NOEXEC - disallow program execution.
> - *	4) MS_SYNCHRONOUS - writes are synced at once.
> - *	5) MS_REMOUNT - alter flags of a mounted FS.
> - *	6) MS_NOSUID - ignore suid and sgid bits.
> - *	7) MS_NOATIME - do not update access times.
> + * Verify mount(2) for various flags.
>   */

Can we please be a bit more verbose here?

> +#include <stdio.h>
> +#include <stdlib.h>
>  #include <sys/types.h>
> -#include <sys/mount.h>
> -#include <sys/stat.h>
>  #include <sys/wait.h>
> -#include <assert.h>
> -#include <errno.h>
> -#include <fcntl.h>
>  #include <pwd.h>
> -#include <unistd.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> -
> -static void setup(void);
> -static void cleanup(void);
> -static int test_rwflag(int, int);
> -
> -char *TCID = "mount03";
> -int TST_TOTAL = 7;
> -
> -#define TEMP_FILE	"temp_file"
> -#define FILE_MODE	(S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)
> -#define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
> -			 S_IXGRP|S_IROTH|S_IXOTH)
> -#define SUID_MODE	(S_ISUID|S_IRUSR|S_IXUSR|S_IXGRP|S_IXOTH)
> -
> -static const char mntpoint[] = "mntpoint";
> -static const char *device;
> -static const char *fs_type;
> -static int fildes;
> -
> -static char write_buffer[BUFSIZ];
> -static char read_buffer[BUFSIZ];
> -static char path_name[PATH_MAX];
> +#include "old_resource.h"
> +#include "tst_test.h"
> +#include "lapi/mount.h"
> +
> +#define MNTPOINT        "mntpoint"
> +#define TESTBIN	"mount03_setuid_test"
> +#define TEST_STR "abcdefghijklmnopqrstuvwxyz"
> +#define FILE_MODE	0644
> +#define SUID_MODE	0511
> +
> +static int otfd;
> +static char wbuf[BUFSIZ];
> +static char rbuf[BUFSIZ];
>  static char file[PATH_MAX];
> +static uid_t nobody_uid;
> +static gid_t nobody_gid;
>  
> -long rwflags[] = {
> -	MS_RDONLY,
> -	MS_NODEV,
> -	MS_NOEXEC,
> -	MS_SYNCHRONOUS,
> -	MS_RDONLY,
> -	MS_NOSUID,
> -	MS_NOATIME,
> -};
> -
> -int main(int argc, char *argv[])
> +static void test_rdonly(void)
>  {
> -	int lc, i;
> -
> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	setup();
> +	snprintf(file, PATH_MAX, "%s/rdonly", MNTPOINT);
> +	TST_EXP_FAIL(otfd = open(file, O_CREAT | O_RDWR, 0700), EROFS);
> +}
>  
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +static void test_nodev(void)
> +{
> +	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);
> +}
>  
> -		tst_count = 0;
> +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);
> +}
>  
> -		for (i = 0; i < TST_TOTAL; ++i) {
> +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);
> +}

This is completely bogus check, this has to work regardless of the
MS_SYNCHRONOUS. The only way how to check MS_SYNCHRONOUS would be
pulling out the device just after write before page cache had a chance
to write out data but not before the disk flushes its caches.

I guess that it may be possible to check this if create a loop device,
mount it MS_SYNCHRONOUS, write to a file on the loop device and check
that the data has been written to the underlying file. But that would
be completely different and quite complex test.

> -			TEST(mount(device, mntpoint, fs_type, rwflags[i],
> -				   NULL));
> +static void test_remount(void)
> +{
> +	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));
> +}
>  
> -			if (TEST_RETURN != 0) {
> -				tst_resm(TFAIL | TTERRNO, "mount(2) failed");
> -				continue;
> -			}
> +static void test_nosuid(void)
> +{
> +	pid_t pid;
> +	int status;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		SAFE_SETGID(nobody_gid);
> +		SAFE_SETREUID(-1, nobody_uid);
> +		SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> +	}
>  
> -			/* Validate the rwflag */
> -			if (test_rwflag(i, lc) == 1)
> -				tst_resm(TFAIL, "mount(2) failed while"
> -					 " validating %ld", rwflags[i]);
> -			else
> -				tst_resm(TPASS, "mount(2) passed with "
> -					 "rwflag = %ld", rwflags[i]);
> +	SAFE_WAITPID(pid, &status, 0);
>  
> -			TEST(tst_umount(mntpoint));
> -			if (TEST_RETURN != 0)
> -				tst_brkm(TBROK | TTERRNO, cleanup,
> -					 "umount(2) failed for %s", mntpoint);
> +	if (WIFEXITED(status)) {
> +		switch (WEXITSTATUS(status)) {
> +		case EXIT_FAILURE:
> +			tst_res(TFAIL, "%s failed", TESTBIN);
> +			return;
> +		case EXIT_SUCCESS:
> +			tst_res(TPASS, "%s passed", TESTBIN);
> +			return;
> +		default:
> +		case TBROK:
> +			break;
>  		}
>  	}
>  
> -	cleanup();
> -	tst_exit();
> +	tst_brk(TBROK, "Child %s", tst_strstatus(status));
>  }
>  
> -/*
> - * test_rwflag(int i, int cnt)
> - * Validate the mount system call for rwflags.
> - */
> -int test_rwflag(int i, int cnt)
> +static void test_noatime(void)
>  {
> -	int ret, fd, pid, status;
> -	char nobody_uid[] = "nobody";
>  	time_t atime;
> -	struct passwd *ltpuser;
> -	struct stat file_stat;
> +	struct stat st;
>  	char readbuf[20];
>  
> -	switch (i) {
> -	case 0:
> -		/* Validate MS_RDONLY flag of mount call */
> -
> -		snprintf(file, PATH_MAX, "%stmp", path_name);
> -		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> -		if (fd == -1) {
> -			if (errno == EROFS) {
> -				return 0;
> -			} else {
> -				tst_resm(TWARN | TERRNO,
> -					 "open didn't fail with EROFS");
> -				return 1;
> -			}
> -		}
> -		close(fd);
> -		return 1;
> -	case 1:
> -		/* Validate MS_NODEV flag of mount call */
> -
> -		snprintf(file, PATH_MAX, "%smynod_%d_%d", path_name, getpid(),
> -			 cnt);
> -		if (mknod(file, S_IFBLK | 0777, 0) == 0) {
> -			fd = open(file, O_RDWR, S_IRWXU);
> -			if (fd == -1) {
> -				if (errno == EACCES) {
> -					return 0;
> -				} else {
> -					tst_resm(TWARN | TERRNO,
> -						 "open didn't fail with EACCES");
> -					return 1;
> -				}
> -			}
> -			close(fd);
> -		} else {
> -			tst_resm(TWARN | TERRNO, "mknod(2) failed to create %s",
> -				 file);
> -			return 1;
> -		}
> -		return 1;
> -	case 2:
> -		/* Validate MS_NOEXEC flag of mount call */
> -
> -		snprintf(file, PATH_MAX, "%stmp1", path_name);
> -		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> -		if (fd == -1) {
> -			tst_resm(TWARN | TERRNO, "opening %s failed", file);
> -		} else {
> -			close(fd);
> -			ret = execlp(file, basename(file), NULL);
> -			if ((ret == -1) && (errno == EACCES))
> -				return 0;
> -		}
> -		return 1;
> -	case 3:
> -		/*
> -		 * 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", path_name, TEMP_FILE);
> -		fildes = open(file, O_RDWR | O_CREAT, FILE_MODE);
> -		if (fildes == -1) {
> -			tst_resm(TWARN | TERRNO,
> -				 "open(%s, O_RDWR|O_CREAT, %#o) failed",
> -				 file, FILE_MODE);
> -			return 1;
> -		}
> +	snprintf(file, PATH_MAX, "%s/noatime", MNTPOINT);
> +	TST_EXP_FD_SILENT(otfd = open(file, O_CREAT | O_RDWR, 0700));
>  
> -		/* Write the buffer data into file */
> -		if (write(fildes, write_buffer, strlen(write_buffer)) !=
> -		    (long)strlen(write_buffer)) {
> -			tst_resm(TWARN | TERRNO, "writing to %s failed", file);
> -			close(fildes);
> -			return 1;
> -		}
> +	SAFE_WRITE(1, otfd, TEST_STR, strlen(TEST_STR));
> +	SAFE_FSTAT(otfd, &st);
> +	atime = st.st_atime;
> +	sleep(1);
>  
> -		/* Set the file ptr to b'nning of file */
> -		if (lseek(fildes, 0, SEEK_SET) < 0) {
> -			tst_resm(TWARN, "lseek() failed on %s, error="
> -				 " %d", file, errno);
> -			close(fildes);
> -			return 1;
> -		}
> -
> -		/* Read the contents of file */
> -		if (read(fildes, read_buffer, sizeof(read_buffer)) > 0) {
> -			if (strcmp(read_buffer, write_buffer)) {
> -				tst_resm(TWARN, "Data read from %s and written "
> -					 "mismatch", file);
> -				close(fildes);
> -				return 1;
> -			} else {
> -				close(fildes);
> -				return 0;
> -			}
> -		} else {
> -			tst_resm(TWARN | TERRNO, "read() Fails on %s", file);
> -			close(fildes);
> -			return 1;
> -		}
> -
> -	case 4:
> -		/* Validate MS_REMOUNT flag of mount call */
> -
> -		TEST(mount(device, mntpoint, fs_type, MS_REMOUNT, NULL));
> -		if (TEST_RETURN != 0) {
> -			tst_resm(TWARN | TTERRNO, "mount(2) failed to remount");
> -			return 1;
> -		} else {
> -			snprintf(file, PATH_MAX, "%stmp2", path_name);
> -			fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> -			if (fd == -1) {
> -				tst_resm(TWARN, "open(%s) on readonly "
> -					 "filesystem passed", file);
> -				return 1;
> -			} else {
> -				close(fd);
> -				return 0;
> -			}
> -		}
> -	case 5:
> -		/* Validate MS_NOSUID flag of mount call */
> -
> -		snprintf(file, PATH_MAX, "%smount03_setuid_test", path_name);
> -
> -		pid = fork();
> -		switch (pid) {
> -		case -1:
> -			tst_resm(TBROK | TERRNO, "fork failed");
> -			return 1;
> -		case 0:
> -			ltpuser = getpwnam(nobody_uid);
> -			if (setreuid(ltpuser->pw_uid, ltpuser->pw_uid) == -1)
> -				tst_resm(TWARN | TERRNO,
> -					 "seteuid() failed to change euid to %d",
> -					 ltpuser->pw_uid);
> -
> -			execlp(file, basename(file), NULL);
> -			exit(1);
> -		default:
> -			waitpid(pid, &status, 0);
> -			if (WIFEXITED(status)) {
> -				/* reset the setup_uid */
> -				if (status)
> -					return 0;
> -			}
> -			return 1;
> -		}
> -	case 6:
> -		/* Validate MS_NOATIME flag of mount call */
> -
> -		snprintf(file, PATH_MAX, "%satime", path_name);
> -		fd = open(file, O_CREAT | O_RDWR, S_IRWXU);
> -		if (fd == -1) {
> -			tst_resm(TWARN | TERRNO, "opening %s failed", file);
> -			return 1;
> -		}
> -
> -		if (write(fd, "TEST_MS_NOATIME", 15) != 15) {
> -			tst_resm(TWARN | TERRNO, "write %s failed", file);
> -			close(fd);
> -			return 1;
> -		}
> -
> -		if (fstat(fd, &file_stat) == -1) {
> -			tst_resm(TWARN | TERRNO, "stat %s failed #1", file);
> -			close(fd);
> -			return 1;
> -		}
> -
> -		atime = file_stat.st_atime;
> -
> -		sleep(1);
> -
> -		if (read(fd, readbuf, sizeof(readbuf)) == -1) {
> -			tst_resm(TWARN | TERRNO, "read %s failed", file);
> -			close(fd);
> -			return 1;
> -		}
> -
> -		if (fstat(fd, &file_stat) == -1) {
> -			tst_resm(TWARN | TERRNO, "stat %s failed #2", file);
> -			close(fd);
> -			return 1;
> -		}
> -		close(fd);
> -
> -		if (file_stat.st_atime != atime) {
> -			tst_resm(TWARN, "access time is updated");
> -			return 1;
> -		}
> -		return 0;
> -	}
> -	return 0;
> +	SAFE_READ(0, otfd, readbuf, sizeof(readbuf));
> +	SAFE_FSTAT(otfd, &st);
> +	TST_EXP_EQ_LI(st.st_atime, 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},
> +};
> +
>  static void setup(void)
>  {
> -	char path[PATH_MAX];
> -	struct stat file_stat;
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> +	struct stat st;
> +	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
>  
> -	tst_require_root();
> +	nobody_uid = ltpuser->pw_uid;
> +	nobody_gid = ltpuser->pw_gid;
>  
> -	tst_tmpdir();
> +	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> +	TST_RESOURCE_COPY(NULL, TESTBIN, file);
>  
> -	fs_type = tst_dev_fs_type();
> -	device = tst_acquire_device(cleanup);
> -
> -	if (!device)
> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> -
> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> +	SAFE_STAT(file, &st);
> +	if (st.st_mode != SUID_MODE)
> +	    SAFE_CHMOD(file, SUID_MODE);
> +}
>  
> -	SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);
> +static void cleanup(void)
> +{
> +	if (otfd >= 0)
> +		SAFE_CLOSE(otfd);
>  
> -	if (getcwd(path_name, sizeof(path_name)) == NULL)
> -		tst_brkm(TBROK, cleanup, "getcwd failed");
> +	if (tst_is_mounted(MNTPOINT))
> +		SAFE_UMOUNT(MNTPOINT);
> +}
>  
> -	if (chmod(path_name, DIR_MODE) != 0)
> -		tst_brkm(TBROK, cleanup, "chmod(%s, %#o) failed",
> -			 path_name, DIR_MODE);
>  
> -	strncpy(path, path_name, PATH_MAX);
> -	snprintf(path_name, PATH_MAX, "%s/%s/", path, mntpoint);
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
>  
> -	SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
> -	TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);
> +	tst_res(TINFO, "Testing flag %s", tc->desc);
>  
> -	snprintf(file, PATH_MAX, "%smount03_setuid_test", path_name);
> -	SAFE_STAT(cleanup, file, &file_stat);
> +	TST_EXP_PASS_SILENT(mount(tst_device->dev, MNTPOINT, tst_device->fs_type,
> +		   tc->flag, NULL));
> +	if (!TST_PASS)
> +		return;
>  
> -	if (file_stat.st_mode != SUID_MODE &&
> -	    chmod(file, SUID_MODE) < 0)
> -		tst_brkm(TBROK, cleanup,
> -			 "setuid for setuid_test failed");
> -	SAFE_UMOUNT(cleanup, mntpoint);
> +	if (tc->test)
> +		tc->test();
>  
> -	TEST_PAUSE;
> +	cleanup();
>  }
>  
> -static void cleanup(void)
> -{
> -	if (device)
> -		tst_release_device(device);
> -
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.test = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.format_device = 1,
> +	.resource_files = (const char *const[]) {
> +		TESTBIN,
> +		NULL,
> +	},
> +	.forks_child = 1,
> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *const []){
> +		"exfat",
> +		"vfat",
> +		"ntfs",
> +		NULL
> +	},
> +};
> -- 
> 2.37.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Cyril,

> > +++ b/testcases/kernel/syscalls/mount/mount03.c
> > @@ -1,389 +1,202 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> >  /*
> > + * Copyright (c) Linux Test Project, 2022
> >   * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
> > - *
> > - * This program is free software; you can redistribute it and/or modify it
> > - * under the terms of version 2 of the GNU General Public License as
> > - * published by the Free Software Foundation.

> This is GPL-2.0 not GPL-2.0-or-later
+1, sorry for overlooking this.

> > -/*
> > - * DESCRIPTION
> > - *	Check for basic mount(2) system call flags.
> > +/*\
> > + * [Description]
> >   *
> > - *	Verify that mount(2) syscall passes for each flag setting and validate
> > - *	the flags
> > - *	1) MS_RDONLY - mount read-only.
> > - *	2) MS_NODEV - disallow access to device special files.
> > - *	3) MS_NOEXEC - disallow program execution.
> > - *	4) MS_SYNCHRONOUS - writes are synced at once.
> > - *	5) MS_REMOUNT - alter flags of a mounted FS.
> > - *	6) MS_NOSUID - ignore suid and sgid bits.
> > - *	7) MS_NOATIME - do not update access times.
> > + * Verify mount(2) for various flags.
> >   */

> Can we please be a bit more verbose here?
Sure, that was my change. Do you want me to put the original description or
would be this enough?

Verify mount(2) run with various flags (e.g. MS_RDONLY, MS_NOEXEC).

=> i.e. what are you missing? I'm not a big fan of listing all features tested,
but if you prefer I'll put the original description.

...
> > +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);
> > +}

> This is completely bogus check, this has to work regardless of the
> MS_SYNCHRONOUS. The only way how to check MS_SYNCHRONOUS would be
> pulling out the device just after write before page cache had a chance
> to write out data but not before the disk flushes its caches.

> I guess that it may be possible to check this if create a loop device,
> mount it MS_SYNCHRONOUS, write to a file on the loop device and check
> that the data has been written to the underlying file. But that would
> be completely different and quite complex test.

OK, I suggest to remove this test and put your suggestion for new to issues.

Also looking to the man page we're missing test for MS_LAZYTIME (since 4.O) and
MS_NOSYMFOLLOW (5.10).

And I'll drop TST_EXP_EQ_STR() unless you think it's useful.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Cyril Hrubis]

Hi!
> > > -/*
> > > - * DESCRIPTION
> > > - *	Check for basic mount(2) system call flags.
> > > +/*\
> > > + * [Description]
> > >   *
> > > - *	Verify that mount(2) syscall passes for each flag setting and validate
> > > - *	the flags
> > > - *	1) MS_RDONLY - mount read-only.
> > > - *	2) MS_NODEV - disallow access to device special files.
> > > - *	3) MS_NOEXEC - disallow program execution.
> > > - *	4) MS_SYNCHRONOUS - writes are synced at once.
> > > - *	5) MS_REMOUNT - alter flags of a mounted FS.
> > > - *	6) MS_NOSUID - ignore suid and sgid bits.
> > > - *	7) MS_NOATIME - do not update access times.
> > > + * Verify mount(2) for various flags.
> > >   */
> 
> > Can we please be a bit more verbose here?
> Sure, that was my change. Do you want me to put the original description or
> would be this enough?
> 
> Verify mount(2) run with various flags (e.g. MS_RDONLY, MS_NOEXEC).
> 
> => i.e. what are you missing? I'm not a big fan of listing all features tested,
> but if you prefer I'll put the original description.

I do not think that listing flags that are tested is pointless, at least
this is supposed to be documentation for the test that is shown on a web
page, it should be a bit more verbose than the single sentence you wrote
there.

> > > +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);
> > > +}
> 
> > This is completely bogus check, this has to work regardless of the
> > MS_SYNCHRONOUS. The only way how to check MS_SYNCHRONOUS would be
> > pulling out the device just after write before page cache had a chance
> > to write out data but not before the disk flushes its caches.
> 
> > I guess that it may be possible to check this if create a loop device,
> > mount it MS_SYNCHRONOUS, write to a file on the loop device and check
> > that the data has been written to the underlying file. But that would
> > be completely different and quite complex test.
> 
> OK, I suggest to remove this test and put your suggestion for new to issues.
> 
> Also looking to the man page we're missing test for MS_LAZYTIME (since 4.O) and
> MS_NOSYMFOLLOW (5.10).

I guess that MS_NOSYMFOLLOW should be easy enough, we just need to
create a symlink to a file and then attempt to open it. We shouldn't end
up with a fd to the symlinked file in that case.

MS_LAZYTIME would be again complicated since that is about deffering
timestamps in memory so it would be similar to MS_SYNCHRONOUS in the
terms of complexity.

> And I'll drop TST_EXP_EQ_STR() unless you think it's useful.

I would follow the usuall, don't add anything that is not used.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi >> Hi!
>>> So I think you should use  SAFE_SETREUID(nobody_uid, nobody_uid);
>>> and then use code as below:
> 
>>> 	if (WIFEXITED(status)) {
>>> 		switch (WEXITSTATUS(status)) {
>>> 		case EXIT_FAILURE:
>>> 			tst_res(TPASS, "%s passed", TESTBIN);
>>> 			return;
>>> 		case EXIT_SUCCESS:
>>> 			tst_res(TFAIL, "%s failed", TESTBIN);
>>> 			return;
>>> 		default:
>>> 		case TBROK:
>>> 			break;
>>> 		}
>>> 	}
> 
>> Can we please stop propagating test success/failure via the exit value?
> 
>> The problem have been solved years ago by the new test library, the
>> child just need to call tst_reinit() and tst_res() will work directly
>> from the child as well. See execlp01.c for example how that works.
> 
> Ah, thanks! I should have read C API docs once more (it's in "1.8 Doing the test
> in the child process").

OK. It is time for me to read C api doc again.

Best Regards
Yang Xu

  At this point there are so many changes => I'll post v4
> and cleanup the child as well.
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

...
> > +++ testcases/kernel/syscalls/mount/mount03.c
...
> >   #define FILE_MODE	0644
> >   #define SUID_MODE	0511
Good catch, SUID_MODE must be 04511.

> Here SUID_MODE miss S_ISUID, that is why this case also pass if we 
> execute a program on filesystem that not mounted with nosuid option.
@Cyril any idea what's wrong here? (not yet tested on tst_reinit()).

> Also, it seems make check code has problem when detecting S_ISUID
Indeed.

> Symbolic permissions 'S_IRUSR|S_IXUSR|S_IXGRP|S_IXOTH' are not 
> preferred. Consider using octal permissions '0511'.
This already been implemented.

> Since we have converted mount03 into new api, mount03_setuid_test.c 
> should also be converted into new spdx.
Planning to, with other required fixes (C API pointed out by Cyril).

> Best Regards
> Yang Xu

> > @@ -74,12 +75,18 @@ static void test_nosuid(void)
> >   {
> >   	pid_t pid;
> >   	int status;
> > +	struct stat st;
> > +
> > +	SAFE_CP(TESTBIN, BIN_PATH);
> > +	SAFE_STAT(BIN_PATH, &st);
> > +	if (st.st_mode != SUID_MODE)
> > +		SAFE_CHMOD(BIN_PATH, SUID_MODE);

> >   	pid = SAFE_FORK();
> >   	if (!pid) {
> >   		SAFE_SETGID(nobody_gid);
> >   		SAFE_SETREUID(-1, nobody_uid);
> > -		SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> > +		SAFE_EXECL(BIN_PATH, TESTBIN, NULL);
> >   	}

> >   	SAFE_WAITPID(pid, &status, 0);
> > @@ -137,18 +144,10 @@ static struct tcase {

> >   static void setup(void)
> >   {
> > -	struct stat st;
> >   	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

> >   	nobody_uid = ltpuser->pw_uid;
> >   	nobody_gid = ltpuser->pw_gid;
> > -
> > -	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > -	SAFE_CP(TESTBIN, file);
> > -
> > -	SAFE_STAT(file, &st);
> > -	if (st.st_mode != SUID_MODE)
> > -	    SAFE_CHMOD(file, SUID_MODE);
> >   }

> >   static void cleanup(void)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Xu,

> ...
> > > +++ testcases/kernel/syscalls/mount/mount03.c
> ...
> > >   #define FILE_MODE	0644
> > >   #define SUID_MODE	0511
> Good catch, SUID_MODE must be 04511.

BTW reading https://lore.kernel.org/ltp/e043db5b-91cd-3e26-d6cd-32189c91518c@fujitsu.com/
do you suggest to use
#define SUID_MODE	0511 | S_ISUID

instead of this?
#define SUID_MODE	04511

Kind regards,
Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[xuyang2018.jy]

Hi Petr

> Hi Xu,
> 
>> ...
>>>> +++ testcases/kernel/syscalls/mount/mount03.c
>> ...
>>>>    #define FILE_MODE	0644
>>>>    #define SUID_MODE	0511
>> Good catch, SUID_MODE must be 04511.
> 
> BTW reading https://lore.kernel.org/ltp/e043db5b-91cd-3e26-d6cd-32189c91518c@fujitsu.com/
> do you suggest to use
> #define SUID_MODE	0511 | S_ISUID
> 
> instead of this?
> #define SUID_MODE	04511

Yes,

Best Regards
Yang Xu
> 
> Kind regards,
> Petr
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

> Hi Petr

> > Hi Xu,

> > ...
> >>>>> -	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.

> >>> old code in setup:
> >>>       fs_type = tst_dev_fs_type();
> >>>       device = tst_acquire_device(cleanup);

> >>>       if (!device)
> >>>           tst_brkm(TCONF, cleanup, "Failed to obtain block device");

> >>>       tst_mkfs(cleanup, device, fs_type, NULL, NULL);

> >>>       SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);

> >>>       SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
> >>>       TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);

> >>> new code:
> >>>       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> >>>       SAFE_CP(TESTBIN, file);

> >>> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
> >>> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?

> >>> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
> >>> (or at least different from the old code).

> >> Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just
> >> create mntpoint instead mount dev to a moutpoint).

> >> So do you will fix this？

> > Yes, see the diff below. I'm waiting little longer if anybody else has some
> > comments before merging it.

> > Thanks for your review.

> >> Best Regards
> >> Yang Xu

> > diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> > index 74b018d78..9c58783d7 100644
> > --- testcases/kernel/syscalls/mount/mount03.c
> > +++ testcases/kernel/syscalls/mount/mount03.c
> > @@ -15,7 +15,6 @@
> >   #include <sys/types.h>
> >   #include <sys/wait.h>
> >   #include <pwd.h>
> > -#include "old_resource.h"
> >   #include "tst_test.h"
> >   #include "lapi/mount.h"

> > @@ -145,7 +144,7 @@ static void setup(void)
> >   	nobody_gid = ltpuser->pw_gid;

> >   	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> > +	SAFE_CP(TESTBIN, file);

> I still think we should test nosuid behaviour on different filesystem 
> like other test function because we have expand it to all filesystems.

> Also include tmpfs, so SAFE_CP should be in test_nosuid function 
> otherwise may hit ENOENT problem.

Actually, I randomly hit ENOENT, when SAFE_CP *is* in test_nosuid().
Not sure what happen (filesystem not synced? sync() does not help).
It works when kept in the setup.

Kind regards,
Petr

> different code as below:

> [root@localhost mount]# git diff .
> diff --git a/testcases/kernel/syscalls/mount/mount03.c 
> b/testcases/kernel/syscalls/mount/mount03.c
> index 74b018d78..b0582c76b 100644
> --- a/testcases/kernel/syscalls/mount/mount03.c
> +++ b/testcases/kernel/syscalls/mount/mount03.c
> @@ -21,6 +21,7 @@

>   #define MNTPOINT        "mntpoint"
>   #define TESTBIN        "mount03_setuid_test"
> +#define BIN_PATH           MNTPOINT"/"TESTBIN
>   #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
>   #define FILE_MODE      0644
>   #define SUID_MODE      0511
> @@ -75,12 +76,19 @@ static void test_nosuid(void)
>   {
>          pid_t pid;
>          int status;
> +       struct stat st;
> +
> +       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> +       SAFE_CP(TESTBIN, file);
> +       SAFE_STAT(file, &st);
> +       if (st.st_mode != SUID_MODE)
> +               SAFE_CHMOD(file, SUID_MODE);

>          pid = SAFE_FORK();
>          if (!pid) {
>                  SAFE_SETGID(nobody_gid);
>                  SAFE_SETREUID(-1, nobody_uid);
> -               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> +               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
>          }

>          SAFE_WAITPID(pid, &status, 0);
> @@ -138,18 +146,10 @@ static struct tcase {

>   static void setup(void)
>   {
> -       struct stat st;
>          struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

>          nobody_uid = ltpuser->pw_uid;
>          nobody_gid = ltpuser->pw_gid;
> -
> -       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> -       TST_RESOURCE_COPY(NULL, TESTBIN, file);
> -
> -       SAFE_STAT(file, &st);
> -       if (st.st_mode != SUID_MODE)
> -           SAFE_CHMOD(file, SUID_MODE);
>   }

>   static void cleanup(void)
> [root@localhost mount]#


> Best Regards
> Yang Xu


> >   	SAFE_STAT(file, &st);
> >   	if (st.st_mode != SUID_MODE)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

> > Hi Petr

> > > Hi Xu,

> > > ...
> > >>>>> -	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.

> > >>> old code in setup:
> > >>>       fs_type = tst_dev_fs_type();
> > >>>       device = tst_acquire_device(cleanup);

> > >>>       if (!device)
> > >>>           tst_brkm(TCONF, cleanup, "Failed to obtain block device");

> > >>>       tst_mkfs(cleanup, device, fs_type, NULL, NULL);

> > >>>       SAFE_MKDIR(cleanup, mntpoint, DIR_MODE);

> > >>>       SAFE_MOUNT(cleanup, device, mntpoint, fs_type, 0, NULL);
> > >>>       TST_RESOURCE_COPY(cleanup, "mount03_setuid_test", path_name);

> > >>> new code:
> > >>>       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > >>>       SAFE_CP(TESTBIN, file);

> > >>> Well, Li in his v2 removed the code because there is .mntpoint = MNTPOINT, in
> > >>> struct tst_test, therefore MNTPOINT is mounted in the filesystem, right?

> > >>> But he also did SAFE_STAT and SAFE_CHMOD on MNTPOINT, which is IMHO wrong
> > >>> (or at least different from the old code).

> > >> Yes, it is wrong. I guess Chen misundertand mntpoint usage(it just
> > >> create mntpoint instead mount dev to a moutpoint).

> > >> So do you will fix this？

> > > Yes, see the diff below. I'm waiting little longer if anybody else has some
> > > comments before merging it.

> > > Thanks for your review.

> > >> Best Regards
> > >> Yang Xu

> > > diff --git testcases/kernel/syscalls/mount/mount03.c testcases/kernel/syscalls/mount/mount03.c
> > > index 74b018d78..9c58783d7 100644
> > > --- testcases/kernel/syscalls/mount/mount03.c
> > > +++ testcases/kernel/syscalls/mount/mount03.c
> > > @@ -15,7 +15,6 @@
> > >   #include <sys/types.h>
> > >   #include <sys/wait.h>
> > >   #include <pwd.h>
> > > -#include "old_resource.h"
> > >   #include "tst_test.h"
> > >   #include "lapi/mount.h"

> > > @@ -145,7 +144,7 @@ static void setup(void)
> > >   	nobody_gid = ltpuser->pw_gid;

> > >   	snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > > -	TST_RESOURCE_COPY(NULL, TESTBIN, file);
> > > +	SAFE_CP(TESTBIN, file);

> > I still think we should test nosuid behaviour on different filesystem 
> > like other test function because we have expand it to all filesystems.

> > Also include tmpfs, so SAFE_CP should be in test_nosuid function 
> > otherwise may hit ENOENT problem.

> Actually, I randomly hit ENOENT, when SAFE_CP *is* in test_nosuid().
> Not sure what happen (filesystem not synced? sync() does not help).
> It works when kept in the setup.
Besides it's really wrong to have it in the setup (mount is dole later in
run()), it also does not work (again, randomly ENOENT).

Kind regards,
Petr

> Kind regards,
> Petr

> > different code as below:

> > [root@localhost mount]# git diff .
> > diff --git a/testcases/kernel/syscalls/mount/mount03.c 
> > b/testcases/kernel/syscalls/mount/mount03.c
> > index 74b018d78..b0582c76b 100644
> > --- a/testcases/kernel/syscalls/mount/mount03.c
> > +++ b/testcases/kernel/syscalls/mount/mount03.c
> > @@ -21,6 +21,7 @@

> >   #define MNTPOINT        "mntpoint"
> >   #define TESTBIN        "mount03_setuid_test"
> > +#define BIN_PATH           MNTPOINT"/"TESTBIN
> >   #define TEST_STR "abcdefghijklmnopqrstuvwxyz"
> >   #define FILE_MODE      0644
> >   #define SUID_MODE      0511
> > @@ -75,12 +76,19 @@ static void test_nosuid(void)
> >   {
> >          pid_t pid;
> >          int status;
> > +       struct stat st;
> > +
> > +       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > +       SAFE_CP(TESTBIN, file);
> > +       SAFE_STAT(file, &st);
> > +       if (st.st_mode != SUID_MODE)
> > +               SAFE_CHMOD(file, SUID_MODE);

> >          pid = SAFE_FORK();
> >          if (!pid) {
> >                  SAFE_SETGID(nobody_gid);
> >                  SAFE_SETREUID(-1, nobody_uid);
> > -               SAFE_EXECLP(TESTBIN, TESTBIN, NULL);
> > +               SAFE_EXECLP(BIN_PATH, TESTBIN, NULL);
> >          }

> >          SAFE_WAITPID(pid, &status, 0);
> > @@ -138,18 +146,10 @@ static struct tcase {

> >   static void setup(void)
> >   {
> > -       struct stat st;
> >          struct passwd *ltpuser = SAFE_GETPWNAM("nobody");

> >          nobody_uid = ltpuser->pw_uid;
> >          nobody_gid = ltpuser->pw_gid;
> > -
> > -       snprintf(file, PATH_MAX, "%s/%s", MNTPOINT, TESTBIN);
> > -       TST_RESOURCE_COPY(NULL, TESTBIN, file);
> > -
> > -       SAFE_STAT(file, &st);
> > -       if (st.st_mode != SUID_MODE)
> > -           SAFE_CHMOD(file, SUID_MODE);
> >   }

> >   static void cleanup(void)
> > [root@localhost mount]#


> > Best Regards
> > Yang Xu


> > >   	SAFE_STAT(file, &st);
> > >   	if (st.st_mode != SUID_MODE)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp