Re: [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS

[Avinesh Kumar <akumar@suse.de>]

Hi Yang Xu,
some comments below

On Monday, April 1, 2024 4:49:48 AM CEST Yang Xu via ltp wrote:
> Add negative cases for unlink(), when errno is EPERM or EROFS.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/unlink/.gitignore |  1 +
>  testcases/kernel/syscalls/unlink/unlink09.c | 86 +++++++++++++++++++++
>  3 files changed, 88 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/unlink/unlink09.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 4ed2b5602..b99ce7170 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1651,6 +1651,7 @@ unlink01 symlink01 -T unlink01
>  unlink05 unlink05
>  unlink07 unlink07
>  unlink08 unlink08
> +unlink09 unlink09
> 
>  #unlinkat test cases
>  unlinkat01 unlinkat01
> diff --git a/testcases/kernel/syscalls/unlink/.gitignore
> b/testcases/kernel/syscalls/unlink/.gitignore index 2e783580d..6038cc29d
> 100644
> --- a/testcases/kernel/syscalls/unlink/.gitignore
> +++ b/testcases/kernel/syscalls/unlink/.gitignore
> @@ -1,3 +1,4 @@
>  /unlink05
>  /unlink07
>  /unlink08
> +/unlink09
> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
> b/testcases/kernel/syscalls/unlink/unlink09.c new file mode 100644
> index 000000000..b7ff94ee6
> --- /dev/null
> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> @@ -0,0 +1,86 @@
> +// 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 unlink(2) fails with
> + *
> + * - EPERM when target file is marked as immutable or append-only
> + * - EROFS when target file is on a read-only filesystem.
> + */
> +
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "lapi/fs.h"
errno.h, unistd.h, ioctl.h and tst_safe_macros.h headers are already included
via tst_test.h
 
> +
> +#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
> +#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
> +#define DIR_EROFS "erofs"
> +#define TEST_EROFS "erofs/test_erofs"
> +
> +static int fd_immutable;
> +static int fd_append_only;
> +
> +static struct test_case_t {
> +	char *filename;
> +	int expected_errno;
> +	char *desc;
> +} tcases[] = {
> +	{TEST_EPERM_IMMUTABLE, EPERM, "target file is immutable"},
> +	{TEST_EPERM_APPEND_ONLY, EPERM, "target file is append-only"},
> +	{TEST_EROFS, EROFS, "target file in read-only filesystem"},
> +};
> +
> +static void setup(void)
> +{
> +	int attr;
> +
> +	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_IMMUTABLE_FL;
> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +
> +	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_APPEND_FL;
> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
We can use SAFE_IOCTL() in all places.
> +}
> +
> +static void cleanup(void)
> +{
> +	int attr;
> +
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_IMMUTABLE_FL;
> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_immutable);
> +
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_APPEND_FL;
> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_append_only);
> +}
> +
> +static void verify_unlink(unsigned int i)
> +{
> +	struct test_case_t *tc = &tcases[i];
> +
> +	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
If for whatever reason, unlink() call passes here, further test iterations
will not work correctly as test file no longer exist. I guess we need to handle
the restoring in this main test function. And cleanup() does not need to reset
any flags as it is called only once after all the iterations.

> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.test = verify_unlink,
> +	.needs_rofs = 1,
> +	.mntpoint = DIR_EROFS,
> +	.needs_root = 1,
> +};


Regards,
Avinesh




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