diff mbox series

[v3] unlinkat: Add negative tests for unlinkat

Message ID 20240417092605.23645-1-xuyang2018.jy@fujitsu.com
State New
Headers show
Series [v3] unlinkat: Add negative tests for unlinkat | expand

Commit Message

Yang Xu \(Fujitsu\) April 17, 2024, 9:26 a.m. UTC 
Add negative cases for unlink(), including following errnos:
EACCES, EFAULT, EISDIR, ENAMETOOLONG ENOENT, ENOTDIR, EPERM, EROFS, EBADF,
EINVAL

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

Comments

Cyril Hrubis June 24, 2024, 11:12 a.m. UTC | #1 
Hi!
Can we move the three negative tests from the unlikat01.c here as well,
so that we have all the negative tests in a single place?

> +static void setup(void)
> +{
> +	int attr;
> +
> +	pw = SAFE_GETPWNAM("nobody");
> +
> +	SAFE_MKDIR(DIR_EACCES_NOWRITE, 0777);
> +	SAFE_TOUCH(DIR_EACCES_NOWRITE "/" TEST_EACCES, 0777, NULL);
> +	SAFE_CHMOD(DIR_EACCES_NOWRITE, 0555);

Can't we pass the 0555 directly to the SAFE_TOUCH()?

> +	SAFE_MKDIR(DIR_EACCES_NOSEARCH, 0777);
> +	SAFE_TOUCH(DIR_EACCES_NOSEARCH "/" TEST_EACCES, 0777, NULL);
> +	SAFE_CHMOD(DIR_EACCES_NOSEARCH, 0666);

Here as well?

> +	SAFE_MKDIR(DIR_NORMAL, 0777);
> +	SAFE_TOUCH(DIR_NORMAL "/" TEST_NORMAL, 0777, NULL);
> +	SAFE_TOUCH(DIR_NORMAL "/" TEST_EFAULT, 0777, NULL);
> +
> +	SAFE_MKDIR(DIR_NORMAL "/" DIR_EISDIR, 0777);
> +
> +	memset(longfilename, '1', PATH_MAX + 1);
> +
> +	SAFE_TOUCH(DIR_NORMAL "/" DIR_ENOTDIR, 0777, NULL);
> +
> +	fd_immutable = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_IMMUTABLE,
> +			O_CREAT, 0777);
> +	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_IMMUTABLE_FL;
> +	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_immutable);

This should be put into a function

static void set_fs_flags(int fd, int flags); and possibly put into the
test library, because this pattern is repeated in several tests.

> +	fd_append_only = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_APPEND_ONLY,
> +			O_CREAT, 0777);
> +	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_APPEND_FL;
> +	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_append_only);
> +
> +	SAFE_TOUCH(DIR_ENOTDIR2, 0777, NULL);
> +}
> +
> +static void cleanup(void)
> +{
> +	int attr;
> +
> +	fd_immutable = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_IMMUTABLE,
> +			O_RDONLY, 0777);
> +	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_IMMUTABLE_FL;
> +	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_immutable);

Same here, this should be put into clear_fs_flags(int fd, int flags)
function.

Or maybe just one function for both:

tst_change_fs_flags(int fd, int flags, bool set);

Where the set will switch between set and clear operations.

> +	fd_append_only = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_APPEND_ONLY,
> +			O_RDONLY, 0777);
> +	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_APPEND_FL;
> +	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_append_only);
> +}
> +
> +static void do_unlinkat(struct test_case_t *tc)
> +{
> +	int attr;
> +	char fullpath[PATH_MAX];
> +	int dirfd = open(tc->dirname, O_DIRECTORY);
> +
> +	if (dirfd < 0) {
> +		if (tc->expected_errno != EBADF) {
> +			/* Special situation: dirfd refers to a file */
> +			if (errno == ENOTDIR)
> +				dirfd = SAFE_OPEN(tc->dirname, O_APPEND);
> +			else {
> +				tst_res(TFAIL | TERRNO, "Cannot open dirfd");
> +				return;
> +			}
> +		}
> +	}

Can't we pass the flags in the testcase structure as well? That way we
would do just:

	dirfd = open(tc->dirname, tc->open_flags);

> +	TST_EXP_FAIL(unlinkat(dirfd, tc->filename, tc->flags),
> +		tc->expected_errno,
> +		"%s", tc->desc);
> +
> +	/* If unlinkat() succeeded unexpectedly, test file should be restored */
> +	if (!TST_RET) {
> +		snprintf(fullpath, sizeof(fullpath), "%s/%s", tc->dirname,
> +			tc->filename);
> +		if (tc->fd) {
> +			*(tc->fd) = SAFE_OPEN(fullpath, O_CREAT, 0600);
> +			if (tc->ioctl_flag) {
> +				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
> +				attr |= tc->ioctl_flag;
> +				SAFE_IOCTL(*(tc->fd), FS_IOC_SETFLAGS, &attr);
> +			}
> +			SAFE_CLOSE(*(tc->fd));
> +		} else {
> +			SAFE_TOUCH(fullpath, 0777, 0);
> +		}
> +	}

Hmm, I'm not sure if this recovery is worth the extra code.

> +	if (dirfd > 0)
> +		SAFE_CLOSE(dirfd);
> +}
> +
> +static void verify_unlinkat(unsigned int i)
> +{
> +	struct test_case_t *tc = &tcases[i];
> +	pid_t pid;
> +
> +	if (tc->user) {
> +		pid = SAFE_FORK();
> +		if (!pid) {
> +			SAFE_SETUID(pw->pw_uid);
> +			do_unlinkat(tc);
> +			exit(0);
> +		}
> +		SAFE_WAITPID(pid, NULL, 0);
> +	} else {
> +		do_unlinkat(tc);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.test = verify_unlinkat,
> +	.needs_rofs = 1,
> +	.mntpoint = DIR_EROFS,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +};
> -- 
> 2.39.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 000c9e536..3521047f4 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1655,6 +1655,7 @@  unlink09 unlink09
 
 #unlinkat test cases
 unlinkat01 unlinkat01
+unlinkat02 unlinkat02
 
 unshare01 unshare01
 unshare02 unshare02
diff --git a/testcases/kernel/syscalls/unlinkat/.gitignore b/testcases/kernel/syscalls/unlinkat/.gitignore
index 76ed551f2..450063051 100644
--- a/testcases/kernel/syscalls/unlinkat/.gitignore
+++ b/testcases/kernel/syscalls/unlinkat/.gitignore
@@ -1 +1,2 @@ 
 /unlinkat01
+/unlinkat02
diff --git a/testcases/kernel/syscalls/unlinkat/unlinkat02.c b/testcases/kernel/syscalls/unlinkat/unlinkat02.c
new file mode 100644
index 000000000..1256bbc57
--- /dev/null
+++ b/testcases/kernel/syscalls/unlinkat/unlinkat02.c
@@ -0,0 +1,234 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that unlinkat(2) fails with
+ *
+ * - EACCES when write access to the directory containing pathname not allowed
+ * - EACCES when one of directories in pathname did not allow search permission
+ * - EFAULT when pathname points outside acessible address space
+ * - EISDIR when pathname refers to a directory
+ * - ENAMETOOLONG when pathname is too long
+ * - ENOENT when a component of the pathname does not exist
+ * - ENOENT when pathname is empty
+ * - ENOTDIR when a component of pathname used as dicrectory is not a directory
+ * - EPERM when file to be unlinked is marked immutable
+ * - EPERM when file to be unlinked is marked append-only
+ * - EROFS when pathname refers to a file on a read-only filesystem
+ * - EBADF when pathname is relative but dirfd is neither AT_FDCWD nor valid
+ * - EINVAL when an invalid flag is specified
+ * - ENOTDIR when pathname is relative and dirfd refers to a file
+ */
+
+#define _GNU_SOURCE
+
+#include <fcntl.h>
+#include <pwd.h>
+#include <sys/ioctl.h>
+#include "tst_test.h"
+#include "lapi/fs.h"
+
+#define DIR_EACCES_NOWRITE "nowrite"
+#define DIR_EACCES_NOSEARCH "nosearch"
+#define TEST_EACCES "test_eacces"
+#define DIR_NORMAL "normal"
+#define TEST_NORMAL "test_normal"
+#define TEST_EFAULT "test_efault"
+#define DIR_EISDIR "isdir"
+#define TEST_ENOENT_NOTEXIST "test_enoent_notexist"
+#define TEST_ENOENT_FILE "test_enoent_file"
+#define TEST_ENOTDIR "enotdir/file"
+#define DIR_ENOTDIR "enotdir"
+#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
+#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
+#define DIR_EROFS "erofs"
+#define TEST_EROFS "test_erofs"
+#define DIR_EBADF "ebadf"
+#define TEST_EBADF "test_ebadf"
+#define DIR_ENOTDIR2 "enotdir2"
+#define TEST_ENOTDIR2 "test_enotdir2"
+
+static struct passwd *pw;
+static char longfilename[PATH_MAX + 1];
+static int fd_immutable;
+static int fd_append_only;
+
+static struct test_case_t {
+	char *dirname;
+	char *filename;
+	int *fd;
+	int ioctl_flag;
+	int flags;
+	int user;
+	int expected_errno;
+	char *desc;
+} tcases[] = {
+	{DIR_EACCES_NOWRITE, TEST_EACCES, NULL, 0, 0, 1, EACCES,
+		"unlinkat() in directory with no write access"},
+	{DIR_EACCES_NOSEARCH, TEST_EACCES, NULL, 0, 0, 1, EACCES,
+		"unlinkat() in directory with no search access"},
+	{DIR_NORMAL, NULL, NULL, 0, 0, 0, EFAULT,
+		"unlinkat() access pathname outside address space"},
+	{DIR_NORMAL, DIR_EISDIR, NULL, 0, 0, 0, EISDIR,
+		"unlinkat() pathname is a directory"},
+	{DIR_NORMAL, longfilename, NULL, 0, 0, 0, ENAMETOOLONG,
+		"unlinkat() pathname is too long"},
+	{DIR_NORMAL, TEST_ENOENT_NOTEXIST, NULL, 0, 0, 0, ENOENT,
+		"unlinkat() pathname does not exist"},
+	{DIR_NORMAL, "", NULL, 0, 0, 0, ENOENT,
+		"unlinkat() pathname is a empty"},
+	{DIR_NORMAL, TEST_ENOTDIR, NULL, 0, 0, 0, ENOTDIR,
+		"unlinkat() component of pathname used as directory "
+		"is not directory"},
+	{DIR_NORMAL, TEST_EPERM_IMMUTABLE, &fd_immutable, FS_IMMUTABLE_FL,
+		0, 0, EPERM,
+		"unlinkat() pathname is immutable"},
+	{DIR_NORMAL, TEST_EPERM_APPEND_ONLY, &fd_append_only, FS_APPEND_FL,
+		0, 0, EPERM,
+		"unlinkat() pathname is append-only"},
+	{DIR_EROFS, TEST_EROFS, NULL, 0, 0, 0, EROFS,
+		"unlinkat() pathname in read-only filesystem"},
+	{DIR_EBADF, TEST_EBADF, NULL, 0, 0, 0, EBADF,
+		"unlinkat() dirfd is not valid"},
+	{DIR_NORMAL, TEST_NORMAL, NULL, 0, -1, 0, EINVAL,
+		"unlinkat() flag is not valid"},
+	{DIR_ENOTDIR2, TEST_ENOTDIR2, NULL, 0, 0, 0, ENOTDIR,
+		"unlinkat() dirfd is not a directory"},
+};
+
+static void setup(void)
+{
+	int attr;
+
+	pw = SAFE_GETPWNAM("nobody");
+
+	SAFE_MKDIR(DIR_EACCES_NOWRITE, 0777);
+	SAFE_TOUCH(DIR_EACCES_NOWRITE "/" TEST_EACCES, 0777, NULL);
+	SAFE_CHMOD(DIR_EACCES_NOWRITE, 0555);
+
+	SAFE_MKDIR(DIR_EACCES_NOSEARCH, 0777);
+	SAFE_TOUCH(DIR_EACCES_NOSEARCH "/" TEST_EACCES, 0777, NULL);
+	SAFE_CHMOD(DIR_EACCES_NOSEARCH, 0666);
+
+	SAFE_MKDIR(DIR_NORMAL, 0777);
+	SAFE_TOUCH(DIR_NORMAL "/" TEST_NORMAL, 0777, NULL);
+	SAFE_TOUCH(DIR_NORMAL "/" TEST_EFAULT, 0777, NULL);
+
+	SAFE_MKDIR(DIR_NORMAL "/" DIR_EISDIR, 0777);
+
+	memset(longfilename, '1', PATH_MAX + 1);
+
+	SAFE_TOUCH(DIR_NORMAL "/" DIR_ENOTDIR, 0777, NULL);
+
+	fd_immutable = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_IMMUTABLE,
+			O_CREAT, 0777);
+	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
+	attr |= FS_IMMUTABLE_FL;
+	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_immutable);
+
+	fd_append_only = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_APPEND_ONLY,
+			O_CREAT, 0777);
+	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
+	attr |= FS_APPEND_FL;
+	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_append_only);
+
+	SAFE_TOUCH(DIR_ENOTDIR2, 0777, NULL);
+}
+
+static void cleanup(void)
+{
+	int attr;
+
+	fd_immutable = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_IMMUTABLE,
+			O_RDONLY, 0777);
+	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
+	attr &= ~FS_IMMUTABLE_FL;
+	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_immutable);
+
+	fd_append_only = SAFE_OPEN(DIR_NORMAL "/" TEST_EPERM_APPEND_ONLY,
+			O_RDONLY, 0777);
+	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
+	attr &= ~FS_APPEND_FL;
+	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_append_only);
+}
+
+static void do_unlinkat(struct test_case_t *tc)
+{
+	int attr;
+	char fullpath[PATH_MAX];
+	int dirfd = open(tc->dirname, O_DIRECTORY);
+
+	if (dirfd < 0) {
+		if (tc->expected_errno != EBADF) {
+			/* Special situation: dirfd refers to a file */
+			if (errno == ENOTDIR)
+				dirfd = SAFE_OPEN(tc->dirname, O_APPEND);
+			else {
+				tst_res(TFAIL | TERRNO, "Cannot open dirfd");
+				return;
+			}
+		}
+	}
+
+	TST_EXP_FAIL(unlinkat(dirfd, tc->filename, tc->flags),
+		tc->expected_errno,
+		"%s", tc->desc);
+
+	/* If unlinkat() succeeded unexpectedly, test file should be restored */
+	if (!TST_RET) {
+		snprintf(fullpath, sizeof(fullpath), "%s/%s", tc->dirname,
+			tc->filename);
+		if (tc->fd) {
+			*(tc->fd) = SAFE_OPEN(fullpath, O_CREAT, 0600);
+			if (tc->ioctl_flag) {
+				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
+				attr |= tc->ioctl_flag;
+				SAFE_IOCTL(*(tc->fd), FS_IOC_SETFLAGS, &attr);
+			}
+			SAFE_CLOSE(*(tc->fd));
+		} else {
+			SAFE_TOUCH(fullpath, 0777, 0);
+		}
+	}
+
+	if (dirfd > 0)
+		SAFE_CLOSE(dirfd);
+}
+
+static void verify_unlinkat(unsigned int i)
+{
+	struct test_case_t *tc = &tcases[i];
+	pid_t pid;
+
+	if (tc->user) {
+		pid = SAFE_FORK();
+		if (!pid) {
+			SAFE_SETUID(pw->pw_uid);
+			do_unlinkat(tc);
+			exit(0);
+		}
+		SAFE_WAITPID(pid, NULL, 0);
+	} else {
+		do_unlinkat(tc);
+	}
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.test = verify_unlinkat,
+	.needs_rofs = 1,
+	.mntpoint = DIR_EROFS,
+	.needs_root = 1,
+	.forks_child = 1,
+};