[LTP] [PATCH] unlinkat: Add negative tests for unlinkat
Yang Xu (Fujitsu)
xuyang2018.jy@fujitsu.com
Thu Apr 11 10:33:56 CEST 2024
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
More information about the ltp
mailing list