diff mbox series

unlinkat: Add negative tests for unlinkat

Message ID 20240401025249.8715-1-xuyang2018.jy@fujitsu.com
State Superseded
Headers show
Series unlinkat: Add negative tests for unlinkat | expand

Commit Message

Yang Xu April 1, 2024, 2:52 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     | 202 ++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 testcases/kernel/syscalls/unlinkat/unlinkat02.c

Comments

Avinesh Kumar April 10, 2024, 11 a.m. UTC | #1
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
Yang Xu April 11, 2024, 8:33 a.m. UTC | #2
Hi Avinesh

Thanks for your comments.

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

I use SAFE_CHMOD to change the mode of test directories in the end.
I think there is no need to consider the umask during creating.

>> +	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
> 
> 
Other comments will be fixed in the v2 patch.

I will send out v2 patch soon.

Best Regards.
Yang Xu
diff mbox series

Patch

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);
+	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);
+}
+
+static void do_unlinkat(struct test_case_t *tc)
+{
+	int dirfd = open(tc->dirname, O_DIRECTORY);
+	/* Special situation when dirfd refers to a file */
+	if (dirfd < 0)
+		dirfd = open(tc->dirname, O_APPEND);
+
+	TST_EXP_FAIL(unlinkat(dirfd, tc->filename, tc->flags),
+		tc->expected_errno,
+		"%s", tc->desc);
+}
+
+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,
+};