[LTP] [PATCH v2 1/2] syscalls/faccessat201: Add new testcase

[LTP] [PATCH v2 1/2] syscalls/faccessat201: Add new testcase

[Yang Xu]

Check the basic functionality of faccessat2().

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                                  |  1 +
 include/lapi/faccessat.h                      | 21 +++++
 include/lapi/syscalls/aarch64.in              |  1 +
 include/lapi/syscalls/arc.in                  |  1 +
 include/lapi/syscalls/arm.in                  |  1 +
 include/lapi/syscalls/hppa.in                 |  1 +
 include/lapi/syscalls/i386.in                 |  1 +
 include/lapi/syscalls/ia64.in                 |  1 +
 include/lapi/syscalls/powerpc.in              |  1 +
 include/lapi/syscalls/powerpc64.in            |  1 +
 include/lapi/syscalls/s390.in                 |  1 +
 include/lapi/syscalls/s390x.in                |  1 +
 include/lapi/syscalls/sh.in                   |  1 +
 include/lapi/syscalls/sparc.in                |  1 +
 include/lapi/syscalls/sparc64.in              |  1 +
 include/lapi/syscalls/x86_64.in               |  1 +
 runtest/syscalls                              |  3 +
 .../kernel/syscalls/faccessat2/.gitignore     |  1 +
 testcases/kernel/syscalls/faccessat2/Makefile |  9 ++
 .../kernel/syscalls/faccessat2/faccessat201.c | 89 +++++++++++++++++++
 20 files changed, 138 insertions(+)
 create mode 100644 include/lapi/faccessat.h
 create mode 100644 testcases/kernel/syscalls/faccessat2/.gitignore
 create mode 100644 testcases/kernel/syscalls/faccessat2/Makefile
 create mode 100644 testcases/kernel/syscalls/faccessat2/faccessat201.c

diff --git a/configure.ac b/configure.ac
index e4aa2cadf..662c4c058 100644
--- a/configure.ac
+++ b/configure.ac
@@ -92,6 +92,7 @@ AC_CHECK_FUNCS_ONCE([ \
     epoll_pwait \
     epoll_pwait2 \
     execveat \
+    faccessat2 \
     fallocate \
     fchownat \
     fsconfig \
diff --git a/include/lapi/faccessat.h b/include/lapi/faccessat.h
new file mode 100644
index 000000000..5b42014d2
--- /dev/null
+++ b/include/lapi/faccessat.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Copyright (c) Linux Test Project, 2003-2023
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+#ifndef FACCESSAT2_H
+#define FACCESSAT2_H
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+#ifndef HAVE_FACCESSAT2
+int faccessat2(int dirfd, const char *pathname, int mode, int flags)
+{
+	return tst_syscall(__NR_faccessat2, dirfd, pathname, mode, flags);
+}
+#endif
+
+#endif /* FACCESSAT2_H */
diff --git a/include/lapi/syscalls/aarch64.in b/include/lapi/syscalls/aarch64.in
index de4ed5fb1..2cb6c2d87 100644
--- a/include/lapi/syscalls/aarch64.in
+++ b/include/lapi/syscalls/aarch64.in
@@ -293,6 +293,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/arc.in b/include/lapi/syscalls/arc.in
index 9f11381db..3e2ee9061 100644
--- a/include/lapi/syscalls/arc.in
+++ b/include/lapi/syscalls/arc.in
@@ -313,6 +313,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/arm.in b/include/lapi/syscalls/arm.in
index 4b0f63a28..7bdbca533 100644
--- a/include/lapi/syscalls/arm.in
+++ b/include/lapi/syscalls/arm.in
@@ -391,6 +391,7 @@ clone3 (__NR_SYSCALL_BASE+435)
 close_range (__NR_SYSCALL_BASE+436)
 openat2 (__NR_SYSCALL_BASE+437)
 pidfd_getfd (__NR_SYSCALL_BASE+438)
+faccessat2 (__NR_SYSCALL_BASE+439)
 epoll_pwait2 (__NR_SYSCALL_BASE+441)
 quotactl_fd (__NR_SYSCALL_BASE+443)
 futex_waitv (__NR_SYSCALL_BASE+449)
diff --git a/include/lapi/syscalls/hppa.in b/include/lapi/syscalls/hppa.in
index b6d32d386..8ebdafafb 100644
--- a/include/lapi/syscalls/hppa.in
+++ b/include/lapi/syscalls/hppa.in
@@ -40,6 +40,7 @@ fsmount 432
 fspick 433
 pidfd_open 434
 close_range 436
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/i386.in b/include/lapi/syscalls/i386.in
index d0e6e9a4b..1472631c4 100644
--- a/include/lapi/syscalls/i386.in
+++ b/include/lapi/syscalls/i386.in
@@ -427,6 +427,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/ia64.in b/include/lapi/syscalls/ia64.in
index 123200624..0ea6e9722 100644
--- a/include/lapi/syscalls/ia64.in
+++ b/include/lapi/syscalls/ia64.in
@@ -340,6 +340,7 @@ pidfd_open 1458
 close_range 1460
 openat2 1461
 pidfd_getfd 1462
+faccessat2 1463
 epoll_pwait2 1465
 quotactl_fd 1467
 futex_waitv 1473
diff --git a/include/lapi/syscalls/powerpc.in b/include/lapi/syscalls/powerpc.in
index d5de621e1..545d9d3d6 100644
--- a/include/lapi/syscalls/powerpc.in
+++ b/include/lapi/syscalls/powerpc.in
@@ -420,6 +420,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in
index d5de621e1..545d9d3d6 100644
--- a/include/lapi/syscalls/powerpc64.in
+++ b/include/lapi/syscalls/powerpc64.in
@@ -420,6 +420,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/s390.in b/include/lapi/syscalls/s390.in
index 6505f3822..7213ac5f8 100644
--- a/include/lapi/syscalls/s390.in
+++ b/include/lapi/syscalls/s390.in
@@ -407,6 +407,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/s390x.in b/include/lapi/syscalls/s390x.in
index bc5d2b34c..879012e2b 100644
--- a/include/lapi/syscalls/s390x.in
+++ b/include/lapi/syscalls/s390x.in
@@ -355,6 +355,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/sh.in b/include/lapi/syscalls/sh.in
index 316ffe5f1..7d5192a27 100644
--- a/include/lapi/syscalls/sh.in
+++ b/include/lapi/syscalls/sh.in
@@ -401,6 +401,7 @@ pidfd_open 434
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/sparc.in b/include/lapi/syscalls/sparc.in
index e0c60a360..91d2fb1c2 100644
--- a/include/lapi/syscalls/sparc.in
+++ b/include/lapi/syscalls/sparc.in
@@ -406,6 +406,7 @@ pidfd_open 434
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/sparc64.in b/include/lapi/syscalls/sparc64.in
index 0acde6856..1f2fc59b7 100644
--- a/include/lapi/syscalls/sparc64.in
+++ b/include/lapi/syscalls/sparc64.in
@@ -371,6 +371,7 @@ pidfd_open 434
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/include/lapi/syscalls/x86_64.in b/include/lapi/syscalls/x86_64.in
index 1863e1df7..dc61aa56e 100644
--- a/include/lapi/syscalls/x86_64.in
+++ b/include/lapi/syscalls/x86_64.in
@@ -348,6 +348,7 @@ clone3 435
 close_range 436
 openat2 437
 pidfd_getfd 438
+faccessat2 439
 epoll_pwait2 441
 quotactl_fd 443
 futex_waitv 449
diff --git a/runtest/syscalls b/runtest/syscalls
index 119710d63..02768c236 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -216,6 +216,9 @@ exit_group01 exit_group01
 #faccessat test cases
 faccessat01 faccessat01
 
+#faccessat2 test cases
+faccessat201 faccessat201
+
 #fallocate test cases
 fallocate01 fallocate01
 fallocate02 fallocate02
diff --git a/testcases/kernel/syscalls/faccessat2/.gitignore b/testcases/kernel/syscalls/faccessat2/.gitignore
new file mode 100644
index 000000000..53f700bac
--- /dev/null
+++ b/testcases/kernel/syscalls/faccessat2/.gitignore
@@ -0,0 +1 @@
+/faccessat201
diff --git a/testcases/kernel/syscalls/faccessat2/Makefile b/testcases/kernel/syscalls/faccessat2/Makefile
new file mode 100644
index 000000000..aaac6b703
--- /dev/null
+++ b/testcases/kernel/syscalls/faccessat2/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+# Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/faccessat2/faccessat201.c b/testcases/kernel/syscalls/faccessat2/faccessat201.c
new file mode 100644
index 000000000..88ee21522
--- /dev/null
+++ b/testcases/kernel/syscalls/faccessat2/faccessat201.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Copyright (c) Linux Test Project, 2003-2023
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Check the basic functionality of faccessat2().
+ *
+ * Minimum Linux version required is v5.8.
+ */
+
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+#include "lapi/faccessat.h"
+
+#define TESTDIR         "faccessat2dir"
+#define TESTFILE        "faccessat2file"
+#define RELPATH         "faccessat2dir/faccessat2file"
+#define TESTSYMLINK     "faccessat2symlink"
+
+static int dir_fd, bad_fd = -1;
+static int atcwd_fd = AT_FDCWD;
+static char *testfile;
+static char *abs_path;
+static char *rel_path;
+static char *sym_path;
+
+static struct tcase {
+	int *fd;
+	char **filename;
+	int flags;
+} tcases[] = {
+	{&dir_fd, &testfile, 0},
+	{&bad_fd, &abs_path, 0},
+	{&atcwd_fd, &rel_path, 0},
+	{&dir_fd, &testfile, AT_EACCESS},
+	{&bad_fd, &abs_path, AT_EACCESS},
+	{&atcwd_fd, &rel_path, AT_EACCESS},
+	{&atcwd_fd, &sym_path, AT_SYMLINK_NOFOLLOW},
+};
+
+static void verify_faccessat2(unsigned int i)
+{
+	struct tcase *tc = &tcases[i];
+
+	TST_EXP_PASS(faccessat2(*tc->fd, *tc->filename, R_OK, tc->flags),
+		     "faccessat2(%d, %s, R_OK, %d)",
+		     *tc->fd, *tc->filename, tc->flags);
+}
+
+static void setup(void)
+{
+	char *tmpdir_path = tst_get_tmpdir();
+
+	abs_path = tst_aprintf("%s/%s", tmpdir_path, RELPATH);
+	free(tmpdir_path);
+
+	SAFE_MKDIR(TESTDIR, 0666);
+	dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
+	SAFE_TOUCH(abs_path, 0444, NULL);
+	SAFE_SYMLINK(abs_path, TESTSYMLINK);
+}
+
+static void cleanup(void)
+{
+	if (dir_fd > -1)
+		SAFE_CLOSE(dir_fd);
+}
+
+static struct tst_test test = {
+	.test = verify_faccessat2,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.bufs = (struct tst_buffers []) {
+		{&testfile, .str = TESTFILE},
+		{&abs_path, .size = sizeof(char)},
+		{&rel_path, .str = RELPATH},
+		{&sym_path, .str = TESTSYMLINK},
+		{},
+	},
+	.needs_tmpdir = 1,
+};
-- 
2.39.1


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

[LTP] [PATCH v2 2/2] syscalls/faccessat202: Add new testcase

[Yang Xu]

Check various errnos for faccessat2().

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                              |   1 +
 .../kernel/syscalls/faccessat2/.gitignore     |   1 +
 .../kernel/syscalls/faccessat2/faccessat202.c | 105 ++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 testcases/kernel/syscalls/faccessat2/faccessat202.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 02768c236..7af2842f3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -218,6 +218,7 @@ faccessat01 faccessat01
 
 #faccessat2 test cases
 faccessat201 faccessat201
+faccessat202 faccessat202
 
 #fallocate test cases
 fallocate01 fallocate01
diff --git a/testcases/kernel/syscalls/faccessat2/.gitignore b/testcases/kernel/syscalls/faccessat2/.gitignore
index 53f700bac..f58f045f4 100644
--- a/testcases/kernel/syscalls/faccessat2/.gitignore
+++ b/testcases/kernel/syscalls/faccessat2/.gitignore
@@ -1 +1,2 @@
 /faccessat201
+/faccessat202
diff --git a/testcases/kernel/syscalls/faccessat2/faccessat202.c b/testcases/kernel/syscalls/faccessat2/faccessat202.c
new file mode 100644
index 000000000..607a615e4
--- /dev/null
+++ b/testcases/kernel/syscalls/faccessat2/faccessat202.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Copyright (c) Linux Test Project, 2003-2023
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test basic error handling of faccessat2 syscall:
+ *
+ * - faccessat2() fails with EFAULT if pathname is a bad pathname point.
+ * - faccessat2() fails with EINVAL if flags is -1.
+ * - faccessat2() fails with EINVAL if mode is -1.
+ * - faccessat2() fails with EBADF if dirfd is -1.
+ * - faccessat2() fails with ENOTDIR if pathname is relative path to a
+ *   file and dir_fd is file descriptor for this file.
+ * - faccessat2() fails with EACCES if flags is AT_EACCESS and not using
+ *   the effective user and group IDs.
+ *
+ * Minimum Linux version required is v5.8.
+ */
+
+#include <pwd.h>
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+#include "lapi/faccessat.h"
+
+#define TESTUSER        "nobody"
+#define TESTDIR         "faccessat2dir"
+#define RELPATH         "faccessat2dir/faccessat2file"
+
+static int fd;
+static int bad_fd = -1;
+static int atcwd_fd = AT_FDCWD;
+static char *bad_path;
+static char *rel_path;
+
+static struct passwd *ltpuser;
+
+static struct tcase {
+	int *fd;
+	char **filename;
+	int mode;
+	int flags;
+	int exp_errno;
+} tcases[] = {
+	{&atcwd_fd, &bad_path, R_OK, 0, EFAULT},
+	{&atcwd_fd, &rel_path, R_OK, -1, EINVAL},
+	{&atcwd_fd, &rel_path, -1, 0, EINVAL},
+	{&bad_fd, &rel_path, R_OK, 0, EBADF},
+	{&fd, &rel_path, R_OK, 0, ENOTDIR},
+	{&atcwd_fd, &rel_path, R_OK, AT_EACCESS, EACCES},
+};
+
+static void verify_faccessat2(unsigned int i)
+{
+	struct tcase *tc = &tcases[i];
+
+	if (tc->exp_errno == EACCES) {
+		if (SAFE_FORK() == 0) {
+			SAFE_SETUID(ltpuser->pw_uid);
+			TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
+				     tc->mode, tc->flags), tc->exp_errno);
+		}
+
+		tst_reap_children();
+	} else {
+		TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
+			     tc->mode, tc->flags), tc->exp_errno);
+	}
+}
+
+static void setup(void)
+{
+	SAFE_MKDIR(TESTDIR, 0666);
+	SAFE_TOUCH(RELPATH, 0444, NULL);
+
+	fd = SAFE_OPEN(RELPATH, O_RDONLY);
+	bad_path = tst_get_bad_addr(NULL);
+
+	ltpuser = SAFE_GETPWNAM(TESTUSER);
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test = verify_faccessat2,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.bufs = (struct tst_buffers []) {
+		{&rel_path, .str = RELPATH},
+		{},
+	},
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.forks_child = 1,
+};
-- 
2.39.1


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

Re: [LTP] [PATCH v2 1/2] syscalls/faccessat201: Add new testcase

[Cyril Hrubis]

Hi!
Pushed with minor changes, thanks.

- we have to include config.h in the lapi/faccessat.h so that we get
  eventually get the macro HAVE_FACCESSAT2 definition (if defined)

- the directory in the test temporary directory must be 0777 otherwise
  the test will fail with EACCESS when executed as non-root

- the abs_path is initialized in the test setup dynamically, it
  shouldn't be initialized in the .bufs structure

Full diff:

diff --git a/include/lapi/faccessat.h b/include/lapi/faccessat.h
index 5b42014d2..05997d975 100644
--- a/include/lapi/faccessat.h
+++ b/include/lapi/faccessat.h
@@ -9,6 +9,7 @@
 #define FACCESSAT2_H

 #include "tst_test.h"
+#include "config.h"
 #include "lapi/syscalls.h"

 #ifndef HAVE_FACCESSAT2
diff --git a/testcases/kernel/syscalls/faccessat2/faccessat201.c b/testcases/kernel/syscalls/faccessat2/faccessat201.c
index 88ee21522..012091752 100644
--- a/testcases/kernel/syscalls/faccessat2/faccessat201.c
+++ b/testcases/kernel/syscalls/faccessat2/faccessat201.c
@@ -61,7 +61,7 @@ static void setup(void)
        abs_path = tst_aprintf("%s/%s", tmpdir_path, RELPATH);
        free(tmpdir_path);

-       SAFE_MKDIR(TESTDIR, 0666);
+       SAFE_MKDIR(TESTDIR, 0777);
        dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
        SAFE_TOUCH(abs_path, 0444, NULL);
        SAFE_SYMLINK(abs_path, TESTSYMLINK);
@@ -80,7 +80,6 @@ static struct tst_test test = {
        .cleanup = cleanup,
        .bufs = (struct tst_buffers []) {
                {&testfile, .str = TESTFILE},
-               {&abs_path, .size = sizeof(char)},
                {&rel_path, .str = RELPATH},
                {&sym_path, .str = TESTSYMLINK},
                {},


-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/2] syscalls/faccessat202: Add new testcase

[Cyril Hrubis]

Hi!
> +static struct passwd *ltpuser;
> +
> +static struct tcase {
> +	int *fd;
> +	char **filename;
> +	int mode;
> +	int flags;
> +	int exp_errno;
> +} tcases[] = {
> +	{&atcwd_fd, &bad_path, R_OK, 0, EFAULT},
> +	{&atcwd_fd, &rel_path, R_OK, -1, EINVAL},
> +	{&atcwd_fd, &rel_path, -1, 0, EINVAL},
> +	{&bad_fd, &rel_path, R_OK, 0, EBADF},
> +	{&fd, &rel_path, R_OK, 0, ENOTDIR},
> +	{&atcwd_fd, &rel_path, R_OK, AT_EACCESS, EACCES},
> +};
> +
> +static void verify_faccessat2(unsigned int i)
> +{
> +	struct tcase *tc = &tcases[i];
> +
> +	if (tc->exp_errno == EACCES) {
> +		if (SAFE_FORK() == 0) {
> +			SAFE_SETUID(ltpuser->pw_uid);
                            ^
			    Should be SAFE_SETEUID() right?

Because the AT_EACESS causes the call to use EUID instead of UID so we
have to change only the EUID and keep the UID to root UID.

And with that we can drop the SAFE_FORK() since we can change EUID back
as long as UID is priviledged, so the code should be:


	if (tc->exp_errno == EACESS)
		SAFE_SETEUID(ltpuser->pw_uid);

	TST_EXP_FAIL(...);

	if (tc->exp_errno == EACESS)
		SAFE_SETEUID(ltpuser->pw_uid);

> +			TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
> +				     tc->mode, tc->flags), tc->exp_errno);
> +		}
> +
> +		tst_reap_children();
> +	} else {
> +		TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
> +			     tc->mode, tc->flags), tc->exp_errno);

Can we get a better message here? As it is it prints
"faccessat2(*tc->fd, *tc->filename, tc->mode, tc->flags) ..."

Which is a bit ugly.

> +	}
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_MKDIR(TESTDIR, 0666);
> +	SAFE_TOUCH(RELPATH, 0444, NULL);
> +
> +	fd = SAFE_OPEN(RELPATH, O_RDONLY);
> +	bad_path = tst_get_bad_addr(NULL);
> +
> +	ltpuser = SAFE_GETPWNAM(TESTUSER);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd > -1)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.test = verify_faccessat2,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.bufs = (struct tst_buffers []) {
> +		{&rel_path, .str = RELPATH},
> +		{},
> +	},
> +	.needs_tmpdir = 1,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +};
> -- 
> 2.39.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/2] syscalls/faccessat202: Add new testcase

[Yang Xu (Fujitsu)]

Hi Cyril
> Hi!
>> +static struct passwd *ltpuser;
>> +
>> +static struct tcase {
>> +	int *fd;
>> +	char **filename;
>> +	int mode;
>> +	int flags;
>> +	int exp_errno;
>> +} tcases[] = {
>> +	{&atcwd_fd, &bad_path, R_OK, 0, EFAULT},
>> +	{&atcwd_fd, &rel_path, R_OK, -1, EINVAL},
>> +	{&atcwd_fd, &rel_path, -1, 0, EINVAL},
>> +	{&bad_fd, &rel_path, R_OK, 0, EBADF},
>> +	{&fd, &rel_path, R_OK, 0, ENOTDIR},
>> +	{&atcwd_fd, &rel_path, R_OK, AT_EACCESS, EACCES},
>> +};
>> +
>> +static void verify_faccessat2(unsigned int i)
>> +{
>> +	struct tcase *tc = &tcases[i];
>> +
>> +	if (tc->exp_errno == EACCES) {
>> +		if (SAFE_FORK() == 0) {
>> +			SAFE_SETUID(ltpuser->pw_uid);
>                              ^
> 			    Should be SAFE_SETEUID() right?
>
> Because the AT_EACESS causes the call to use EUID instead of UID so we
> have to change only the EUID and keep the UID to root UID.
>
> And with that we can drop the SAFE_FORK() since we can change EUID back
> as long as UID is priviledged, so the code should be:
>
>
> 	if (tc->exp_errno == EACESS)
> 		SAFE_SETEUID(ltpuser->pw_uid);
>
> 	TST_EXP_FAIL(...);
>
> 	if (tc->exp_errno == EACESS)
> 		SAFE_SETEUID(ltpuser->pw_uid);

Thank you for your suggestion, but i think " SAFE_SETEUID(ltpuser->pw_uid)"

should be changed to "SAFE_SETEUID(0)" to change EUID back.

>> +			TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
>> +				     tc->mode, tc->flags), tc->exp_errno);
>> +		}
>> +
>> +		tst_reap_children();
>> +	} else {
>> +		TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
>> +			     tc->mode, tc->flags), tc->exp_errno);
> Can we get a better message here? As it is it prints
> "faccessat2(*tc->fd, *tc->filename, tc->mode, tc->flags) ..."
>
> Which is a bit ugly.

Because i used tst_get_bad_addr to test a bad pathname point ,

so if i output relevant information, the test will be killed by SIGSEGV.

In the v3 version, I will change it to this:

"TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename, tc->mode, tc->flags),

                         tc->exp_errno, "faccessat2()")"

How about this?

>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	SAFE_MKDIR(TESTDIR, 0666);
>> +	SAFE_TOUCH(RELPATH, 0444, NULL);
>> +
>> +	fd = SAFE_OPEN(RELPATH, O_RDONLY);
>> +	bad_path = tst_get_bad_addr(NULL);
>> +
>> +	ltpuser = SAFE_GETPWNAM(TESTUSER);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (fd > -1)
>> +		SAFE_CLOSE(fd);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test = verify_faccessat2,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.bufs = (struct tst_buffers []) {
>> +		{&rel_path, .str = RELPATH},
>> +		{},
>> +	},
>> +	.needs_tmpdir = 1,
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +};
>> -- 
>> 2.39.1
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Thanks for your review.

Best Regards

Yang Xu

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

Re: [LTP] [PATCH v2 2/2] syscalls/faccessat202: Add new testcase

[Cyril Hrubis]

Hi!
> 
> Thank you for your suggestion, but i think " SAFE_SETEUID(ltpuser->pw_uid)"
> 
> should be changed to "SAFE_SETEUID(0)" to change EUID back.

Right, that was copy&paste typo.

> >> +			TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
> >> +				     tc->mode, tc->flags), tc->exp_errno);
> >> +		}
> >> +
> >> +		tst_reap_children();
> >> +	} else {
> >> +		TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename,
> >> +			     tc->mode, tc->flags), tc->exp_errno);
> > Can we get a better message here? As it is it prints
> > "faccessat2(*tc->fd, *tc->filename, tc->mode, tc->flags) ..."
> >
> > Which is a bit ugly.
> 
> Because i used tst_get_bad_addr to test a bad pathname point ,
> 
> so if i output relevant information, the test will be killed by SIGSEGV.
> 
> In the v3 version, I will change it to this:
> 
> "TST_EXP_FAIL(faccessat2(*tc->fd, *tc->filename, tc->mode, tc->flags),
> 
>                          tc->exp_errno, "faccessat2()")"
> 
> How about this?

Either that or we can add short description to the test structure and
pass TST_EXP_FAIL(..., "faccessat2() %s", tc->desc)

At least some cases would be better with description, for instance for
the euid test the description should be something as
"with AT_EACCESS and unpriviledged EUID"

-- 
Cyril Hrubis
chrubis@suse.cz

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