[LTP] [PATCH] unlink09: Fix open syscall flags

[LTP] [PATCH] unlink09: Fix open syscall flags

[Avinesh Kumar]

Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/unlink/unlink09.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
index cc4b4a07e..405deb05f 100644
--- a/testcases/kernel/syscalls/unlink/unlink09.c
+++ b/testcases/kernel/syscalls/unlink/unlink09.c
@@ -43,12 +43,12 @@ static void setup(void)
 {
 	int attr;
 
-	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
+	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_RDWR | O_CREAT, 0600);
 	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
 	attr |= FS_IMMUTABLE_FL;
 	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
 
-	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
+	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_RDWR | O_CREAT, 0600);
 	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
 	attr |= FS_APPEND_FL;
 	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
@@ -79,7 +79,7 @@ static void verify_unlink(unsigned int i)
 	/* If unlink() succeeded unexpectedly, test file should be restored. */
 	if (!TST_RET) {
 		if (tc->fd) {
-			*(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
+			*(tc->fd) = SAFE_OPEN(tc->filename, O_RDWR | O_CREAT, 0600);
 			if (tc->flag) {
 				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
 				attr |= tc->flag;
-- 
2.45.1


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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Sebastian Chlad]

Hi Avinesh,

Could you please amend the git commit message to explain why that flag
would be needed?

Thanks,
Seb/

On Sat, 1 Jun 2024 at 21:51, Avinesh Kumar <akumar@suse.de> wrote:

> Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>  testcases/kernel/syscalls/unlink/unlink09.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
> b/testcases/kernel/syscalls/unlink/unlink09.c
> index cc4b4a07e..405deb05f 100644
> --- a/testcases/kernel/syscalls/unlink/unlink09.c
> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> @@ -43,12 +43,12 @@ static void setup(void)
>  {
>         int attr;
>
> -       fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> +       fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_RDWR | O_CREAT,
> 0600);
>         SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
>         attr |= FS_IMMUTABLE_FL;
>         SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
>
> -       fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> +       fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_RDWR |
> O_CREAT, 0600);
>         SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
>         attr |= FS_APPEND_FL;
>         SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
> @@ -79,7 +79,7 @@ static void verify_unlink(unsigned int i)
>         /* If unlink() succeeded unexpectedly, test file should be
> restored. */
>         if (!TST_RET) {
>                 if (tc->fd) {
> -                       *(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
> +                       *(tc->fd) = SAFE_OPEN(tc->filename, O_RDWR |
> O_CREAT, 0600);
>                         if (tc->flag) {
>                                 SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS,
> &attr);
>                                 attr |= tc->flag;
> --
> 2.45.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>

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

[LTP] [PATCH] unlink09: Fix open syscall flags

[Avinesh Kumar]

In the SAFE_OPEN() calls, we missed to include any of the mandatory
flags for open syscall:  O_RDONLY,  O_WRONLY,  or  O_RDWR

Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/unlink/unlink09.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
index cc4b4a07e..405deb05f 100644
--- a/testcases/kernel/syscalls/unlink/unlink09.c
+++ b/testcases/kernel/syscalls/unlink/unlink09.c
@@ -43,12 +43,12 @@ static void setup(void)
 {
 	int attr;
 
-	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
+	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_RDWR | O_CREAT, 0600);
 	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
 	attr |= FS_IMMUTABLE_FL;
 	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
 
-	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
+	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_RDWR | O_CREAT, 0600);
 	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
 	attr |= FS_APPEND_FL;
 	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
@@ -79,7 +79,7 @@ static void verify_unlink(unsigned int i)
 	/* If unlink() succeeded unexpectedly, test file should be restored. */
 	if (!TST_RET) {
 		if (tc->fd) {
-			*(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
+			*(tc->fd) = SAFE_OPEN(tc->filename, O_RDWR | O_CREAT, 0600);
 			if (tc->flag) {
 				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
 				attr |= tc->flag;
-- 
2.45.1


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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Andrea Cervesato via ltp]

Hi!

thanks for fixing the commit message and the test. According with the 
open() documentation an access flag
is a must and that's one thing that makes this test wrong.

LGTM

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 6/3/24 14:46, Avinesh Kumar wrote:
> In the SAFE_OPEN() calls, we missed to include any of the mandatory
> flags for open syscall:  O_RDONLY,  O_WRONLY,  or  O_RDWR
>
> Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>   testcases/kernel/syscalls/unlink/unlink09.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
> index cc4b4a07e..405deb05f 100644
> --- a/testcases/kernel/syscalls/unlink/unlink09.c
> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> @@ -43,12 +43,12 @@ static void setup(void)
>   {
>   	int attr;
>   
> -	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> +	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_RDWR | O_CREAT, 0600);
>   	SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
>   	attr |= FS_IMMUTABLE_FL;
>   	SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
>   
> -	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> +	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_RDWR | O_CREAT, 0600);
>   	SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
>   	attr |= FS_APPEND_FL;
>   	SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
> @@ -79,7 +79,7 @@ static void verify_unlink(unsigned int i)
>   	/* If unlink() succeeded unexpectedly, test file should be restored. */
>   	if (!TST_RET) {
>   		if (tc->fd) {
> -			*(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
> +			*(tc->fd) = SAFE_OPEN(tc->filename, O_RDWR | O_CREAT, 0600);
>   			if (tc->flag) {
>   				SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS, &attr);
>   				attr |= tc->flag;

Andrea


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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Sebastian Chlad]

+1

Reviewed-by: Sebastian Chlad <sebastianchlad@gmail.com>

On Mon, 3 Jun 2024 at 15:49, Andrea Cervesato via ltp <ltp@lists.linux.it>
wrote:

> Hi!
>
> thanks for fixing the commit message and the test. According with the
> open() documentation an access flag
> is a must and that's one thing that makes this test wrong.
>
> LGTM
>
> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
>
> On 6/3/24 14:46, Avinesh Kumar wrote:
> > In the SAFE_OPEN() calls, we missed to include any of the mandatory
> > flags for open syscall:  O_RDONLY,  O_WRONLY,  or  O_RDWR
> >
> > Fixes: 2cf78f47a6 (unlink: Add error tests for EPERM and EROFS)
> > Signed-off-by: Avinesh Kumar <akumar@suse.de>
> > ---
> >   testcases/kernel/syscalls/unlink/unlink09.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
> b/testcases/kernel/syscalls/unlink/unlink09.c
> > index cc4b4a07e..405deb05f 100644
> > --- a/testcases/kernel/syscalls/unlink/unlink09.c
> > +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> > @@ -43,12 +43,12 @@ static void setup(void)
> >   {
> >       int attr;
> >
> > -     fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> > +     fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_RDWR | O_CREAT,
> 0600);
> >       SAFE_IOCTL(fd_immutable, FS_IOC_GETFLAGS, &attr);
> >       attr |= FS_IMMUTABLE_FL;
> >       SAFE_IOCTL(fd_immutable, FS_IOC_SETFLAGS, &attr);
> >
> > -     fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> > +     fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_RDWR |
> O_CREAT, 0600);
> >       SAFE_IOCTL(fd_append_only, FS_IOC_GETFLAGS, &attr);
> >       attr |= FS_APPEND_FL;
> >       SAFE_IOCTL(fd_append_only, FS_IOC_SETFLAGS, &attr);
> > @@ -79,7 +79,7 @@ static void verify_unlink(unsigned int i)
> >       /* If unlink() succeeded unexpectedly, test file should be
> restored. */
> >       if (!TST_RET) {
> >               if (tc->fd) {
> > -                     *(tc->fd) = SAFE_OPEN(tc->filename, O_CREAT, 0600);
> > +                     *(tc->fd) = SAFE_OPEN(tc->filename, O_RDWR |
> O_CREAT, 0600);
> >                       if (tc->flag) {
> >                               SAFE_IOCTL(*(tc->fd), FS_IOC_GETFLAGS,
> &attr);
> >                               attr |= tc->flag;
>
> Andrea
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>

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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Petr Vorel]

Hi all,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

I suppose with this fix Andrea's extra check is not needed, right?
But sure, it would not harm anyway.

It'd be still interesting to know what exactly triggered the problem.

https://patchwork.ozlabs.org/project/ltp/patch/20240604-unlink09-v1-1-dfd8e3e1cb2b@suse.com/

Kind regards,
Petr

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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Andrea Cervesato via ltp]

Hi,

Unfortunately this patch is not working, due to the fact test requires 
to check if filesystem supports inode attributes.
OPEN_CREAT() is more appropriate in this case and please refer to the 
following patch instead, since it's fixing the issue:

https://patchwork.ozlabs.org/project/ltp/patch/20240604-unlink09-v1-1-dfd8e3e1cb2b@suse.com/

Andrea

On 6/5/24 09:11, Petr Vorel wrote:
> Hi all,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> I suppose with this fix Andrea's extra check is not needed, right?
> But sure, it would not harm anyway.
>
> It'd be still interesting to know what exactly triggered the problem.
>
> https://patchwork.ozlabs.org/project/ltp/patch/20240604-unlink09-v1-1-dfd8e3e1cb2b@suse.com/
>
> Kind regards,
> Petr



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

Re: [LTP] [PATCH] unlink09: Fix open syscall flags

[Petr Vorel]

Hi all,

just to document, we agreed in discussion of alternative patch, that more fixes
are needed, thus setting this in patchwork as changes requested.

https://lore.kernel.org/ltp/ad6558e0-e834-4b35-923a-7b519384f436@suse.cz/
https://patchwork.ozlabs.org/project/ltp/patch/20240604-unlink09-v1-1-dfd8e3e1cb2b@suse.com/

Kind regards,
Petr

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