[LTP] [PATCH] unlinkat: Add negative tests for unlinkat

[Avinesh Kumar]

Hi Yang Xu,
Please see comments below

On Monday, April 1, 2024 4:52:49 AM CEST Yang Xu via ltp wrote:
> 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     | 202 ++++++++++++++++++
>  3 files changed, 204 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/unlinkat/unlinkat02.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index b99ce7170..ed5eab1a9 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..87e6dc704
> --- /dev/null
> +++ b/testcases/kernel/syscalls/unlinkat/unlinkat02.c
> @@ -0,0 +1,202 @@
> +// 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 <errno.h>
> +#include <fcntl.h>
> +#include <pwd.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include "tst_test.h"
> +#include "tst_safe_macros.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 flags;
> +	int user;
> +	int expected_errno;
> +	char *desc;
> +} tcases[] = {
> +	{DIR_EACCES_NOWRITE, TEST_EACCES, 0, 1, EACCES,
> +		"unlinkat() in directory with no write access"},
> +	{DIR_EACCES_NOSEARCH, TEST_EACCES, 0, 1, EACCES,
> +		"unlinkat() in directory with no search access"},
> +	{DIR_NORMAL, NULL, 0, 0, EFAULT,
> +		"unlinkat() access pathname outside address space"},
> +	{DIR_NORMAL, DIR_EISDIR, 0, 0, EISDIR,
> +		"unlinkat() pathname is a directory"},
> +	{DIR_NORMAL, longfilename, 0, 0, ENAMETOOLONG,
> +		"unlinkat() pathname is too long"},
> +	{DIR_NORMAL, TEST_ENOENT_NOTEXIST, 0, 0, ENOENT,
> +		"unlinkat() pathname does not exist"},
> +	{DIR_NORMAL, "", 0, 0, ENOENT,
> +		"unlinkat() pathname is a empty"},
> +	{DIR_NORMAL, TEST_ENOTDIR, 0, 0, ENOTDIR,
> +		"unlinkat() component of pathname used as directory "
> +		"is not directory"},
> +	{DIR_NORMAL, TEST_EPERM_IMMUTABLE, 0, 0, EPERM,
> +		"unlinkat() pathname is immutable"},
> +	{DIR_NORMAL, TEST_EPERM_APPEND_ONLY, 0, 0, EPERM,
> +		"unlinkat() pathname is append-only"},
> +	{DIR_EROFS, TEST_EROFS, 0, 0, EROFS,
> +		"unlinkat() pathname in read-only filesystem"},
> +	{DIR_EBADF, TEST_EBADF, 0, 0, EBADF,
> +		"unlinkat() dirfd is not valid"},
> +	{DIR_NORMAL, TEST_NORMAL, -1, 0, EINVAL,
> +		"unlinkat() flag is not valid"},
> +	{DIR_ENOTDIR2, TEST_ENOTDIR2, 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);
I am not sure if we need to consider setting the umask to 0 before creating
the test directories, without that directories will not have the mode bits as
requested.
> +	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);
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_IMMUTABLE_FL;
> +	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);
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_IMMUTABLE_FL;
> +	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);
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_IMMUTABLE_FL;
> +	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);
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_IMMUTABLE_FL;
> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_append_only);

All the restoring should not be part of cleanup, instead it must be handled
in the main test function. setup() and cleanup() is called just once.
So if somehow unlinkat() call doesn't fail, all the further iterations will
be broken.

> +}
> +
> +static void do_unlinkat(struct test_case_t *tc)
> +{
> +	int dirfd = open(tc->dirname, O_DIRECTORY);
> +	/* Special situation when dirfd refers to a file */
I think we should also check the errno == ENOTDIR to make sure open()
is not failing for some other reason.
> +	if (dirfd < 0)
> +		dirfd = open(tc->dirname, O_APPEND);
> +
> +	TST_EXP_FAIL(unlinkat(dirfd, tc->filename, tc->flags),
> +		tc->expected_errno,
> +		"%s", tc->desc);
dirfd need to be closed after each iteration, tests starts failing with EBADF
when we exhaust the limit for max open fds, try running with '-i 1100' option.
> +}
> +
> +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,
> +};


Regards,
Avinesh