[LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Nobuhiro Iwamatsu]

memfd_secret is a syscall added since 5.14. On earlier kernels, tests such
as accept03, readahead01 and splice07 that use memfd_secret fail.
This adds a kernel version check to the tst_fd library when running tests using
memfd_secret.

Test log on linux-5.10.162/arm32 with version 20240524:
```
$ ./testcases/kernel/syscalls/accept/accept03
tst_test.c:1733: TINFO: LTP version: 20240524
tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
tst_test.c:1677: TBROK: Test killed by SIGILL!

Summary:
passed   20
failed   0
broken   1
skipped  2
warnings 0
```

Closed: #1145
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 lib/tst_fd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/tst_fd.c b/lib/tst_fd.c
index 6538a098c..53f583fa0 100644
--- a/lib/tst_fd.c
+++ b/lib/tst_fd.c
@@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)
 
 static void open_memfd_secret(struct tst_fd *fd)
 {
+	if ((tst_kvercmp(5, 14, 0)) < 0) {
+		tst_res(TINFO, "accept() on %s: Linux kernel version is before than v5.14", tst_fd_desc(fd));
+		errno = ENOSYS;
+		goto skip;
+	}
+
 	fd->fd = syscall(__NR_memfd_secret, 0);
+
 	if (fd->fd < 0) {
+skip:
 		tst_res(TCONF | TERRNO,
 			"Skipping %s", tst_fd_desc(fd));
 	}
-- 
2.43.0



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

[LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Nobuhiro Iwamatsu]

memfd_secret is a syscall added since 5.14. On earlier kernels, tests such
as accept03, readahead01 and splice07 that use memfd_secret fail.
This adds a kernel version check to the tst_fd library when running tests using
memfd_secret.

Test log on linux-5.10.162/arm32 with version 20240524:
```
$ ./testcases/kernel/syscalls/accept/accept03
tst_test.c:1733: TINFO: LTP version: 20240524
tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
tst_test.c:1677: TBROK: Test killed by SIGILL!

Summary:
passed   20
failed   0
broken   1
skipped  2
warnings 0
```

Closed: #1145
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 lib/tst_fd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/tst_fd.c b/lib/tst_fd.c
index 6538a098c..53f583fa0 100644
--- a/lib/tst_fd.c
+++ b/lib/tst_fd.c
@@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)
 
 static void open_memfd_secret(struct tst_fd *fd)
 {
+	if ((tst_kvercmp(5, 14, 0)) < 0) {
+		tst_res(TINFO, "accept() on %s: Linux kernel version is before than v5.14", tst_fd_desc(fd));
+		errno = ENOSYS;
+		goto skip;
+	}
+
 	fd->fd = syscall(__NR_memfd_secret, 0);
+
 	if (fd->fd < 0) {
+skip:
 		tst_res(TCONF | TERRNO,
 			"Skipping %s", tst_fd_desc(fd));
 	}
-- 
2.43.0



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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Cyril Hrubis]

Hi!
> memfd_secret is a syscall added since 5.14. On earlier kernels, tests such
> as accept03, readahead01 and splice07 that use memfd_secret fail.
> This adds a kernel version check to the tst_fd library when running tests using
> memfd_secret.
> 
> Test log on linux-5.10.162/arm32 with version 20240524:
> ```
> $ ./testcases/kernel/syscalls/accept/accept03
> tst_test.c:1733: TINFO: LTP version: 20240524
> tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
> accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
> accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
> tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
> accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
> tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
> accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
> accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
> accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
> tst_test.c:1677: TBROK: Test killed by SIGILL!

This looks like a bug either in kernel or libc.

> Summary:
> passed   20
> failed   0
> broken   1
> skipped  2
> warnings 0
> ```
> 
> Closed: #1145
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
>  lib/tst_fd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/tst_fd.c b/lib/tst_fd.c
> index 6538a098c..53f583fa0 100644
> --- a/lib/tst_fd.c
> +++ b/lib/tst_fd.c
> @@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)
>  
>  static void open_memfd_secret(struct tst_fd *fd)
>  {
> +	if ((tst_kvercmp(5, 14, 0)) < 0) {
> +		tst_res(TINFO, "accept() on %s: Linux kernel version is before than v5.14", tst_fd_desc(fd));
> +		errno = ENOSYS;
> +		goto skip;
> +	}
> +
>  	fd->fd = syscall(__NR_memfd_secret, 0);
> +
>  	if (fd->fd < 0) {
> +skip:
>  		tst_res(TCONF | TERRNO,
>  			"Skipping %s", tst_fd_desc(fd));
>  	}

And this looks like you are working around the bug.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[nobuhiro1.iwamatsu]

Hi,

Thanks for your review.

> -----Original Message-----
> From: Cyril Hrubis <chrubis@suse.cz>
> Sent: Tuesday, June 18, 2024 7:16 PM
> To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
> <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: ltp@lists.linux.it
> Subject: Re: [PATCH] lib: tst_fd: Add kernel version check to memfd_secret
> 
> Hi!
> > memfd_secret is a syscall added since 5.14. On earlier kernels, tests
> > such as accept03, readahead01 and splice07 that use memfd_secret fail.
> > This adds a kernel version check to the tst_fd library when running
> > tests using memfd_secret.
> >
> > Test log on linux-5.10.162/arm32 with version 20240524:
> > ```
> > $ ./testcases/kernel/syscalls/accept/accept03
> > tst_test.c:1733: TINFO: LTP version: 20240524
> > tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
> > accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
> > accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
> > tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
> > accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
> > tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
> > accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
> > accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
> > accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
> > tst_test.c:1677: TBROK: Test killed by SIGILL!
> 
> This looks like a bug either in kernel or libc.

This is caused by __NR_memfd_secure being defined as -1 (0xffffffff)and "Illegal instruction" 
occurs when syscall() is executed. And this problem does not occur on x86_64. 
I cannot decide if this is a bug or not. I can't decide if this is a bug or not, because this behavior has
existed for a long time.

> 
> > Summary:
> > passed   20
> > failed   0
> > broken   1
> > skipped  2
> > warnings 0
> > ```
> >
> > Closed: #1145
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > ---
> >  lib/tst_fd.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/tst_fd.c b/lib/tst_fd.c index 6538a098c..53f583fa0
> > 100644
> > --- a/lib/tst_fd.c
> > +++ b/lib/tst_fd.c
> > @@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)
> >
> >  static void open_memfd_secret(struct tst_fd *fd)  {
> > +	if ((tst_kvercmp(5, 14, 0)) < 0) {
> > +		tst_res(TINFO, "accept() on %s: Linux kernel version is before
> than v5.14", tst_fd_desc(fd));
> > +		errno = ENOSYS;
> > +		goto skip;
> > +	}
> > +
> >  	fd->fd = syscall(__NR_memfd_secret, 0);
> > +
> >  	if (fd->fd < 0) {
> > +skip:
> >  		tst_res(TCONF | TERRNO,
> >  			"Skipping %s", tst_fd_desc(fd));
> >  	}
> 
> And this looks like you are working around the bug.

Your point is correct...
I would suggest using tst_syscall() to check for syscall undefined instead
of this modification. How about this modification? 

```
--- a/lib/tst_fd.c
+++ b/lib/tst_fd.c
@@ -255,7 +255,8 @@ static void open_memfd(struct tst_fd *fd)

 static void open_memfd_secret(struct tst_fd *fd)
 {
-       fd->fd = syscall(__NR_memfd_secret, 0);
+       fd->fd = tst_syscall(__NR_memfd_secret, 0);
        if (fd->fd < 0) {
                tst_res(TCONF | TERRNO,
                        "Skipping %s", tst_fd_desc(fd));
```


Best regards,
  Nobuhiro


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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Petr Vorel]

> Hi,

> Thanks for your review.

> > -----Original Message-----
> > From: Cyril Hrubis <chrubis@suse.cz>
> > Sent: Tuesday, June 18, 2024 7:16 PM
> > To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
> > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Cc: ltp@lists.linux.it
> > Subject: Re: [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

> > Hi!
> > > memfd_secret is a syscall added since 5.14. On earlier kernels, tests
> > > such as accept03, readahead01 and splice07 that use memfd_secret fail.
> > > This adds a kernel version check to the tst_fd library when running
> > > tests using memfd_secret.

> > > Test log on linux-5.10.162/arm32 with version 20240524:
> > > ```
> > > $ ./testcases/kernel/syscalls/accept/accept03
> > > tst_test.c:1733: TINFO: LTP version: 20240524
> > > tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
> > > accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
> > > accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
> > > tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
> > > accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
> > > tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
> > > accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
> > > accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
> > > accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
> > > tst_test.c:1677: TBROK: Test killed by SIGILL!

> > This looks like a bug either in kernel or libc.

> This is caused by __NR_memfd_secure being defined as -1 (0xffffffff)and "Illegal instruction" 
> occurs when syscall() is executed. And this problem does not occur on x86_64. 
> I cannot decide if this is a bug or not. I can't decide if this is a bug or not, because this behavior has
> existed for a long time.

Interesting. But it'd be good to discuss it, right? In case there is something
to improve. Cc linux-arm-kernel ML.

> > > Summary:
> > > passed   20
> > > failed   0
> > > broken   1
> > > skipped  2
> > > warnings 0
> > > ```

> > > Closed: #1145
> > > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > ---
> > >  lib/tst_fd.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)

> > > diff --git a/lib/tst_fd.c b/lib/tst_fd.c index 6538a098c..53f583fa0
> > > 100644
> > > --- a/lib/tst_fd.c
> > > +++ b/lib/tst_fd.c
> > > @@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)

> > >  static void open_memfd_secret(struct tst_fd *fd)  {
> > > +	if ((tst_kvercmp(5, 14, 0)) < 0) {
> > > +		tst_res(TINFO, "accept() on %s: Linux kernel version is before
> > than v5.14", tst_fd_desc(fd));


> > > +		errno = ENOSYS;
> > > +		goto skip;
> > > +	}
> > > +
> > >  	fd->fd = syscall(__NR_memfd_secret, 0);
> > > +
> > >  	if (fd->fd < 0) {
> > > +skip:
> > >  		tst_res(TCONF | TERRNO,
> > >  			"Skipping %s", tst_fd_desc(fd));
> > >  	}

> > And this looks like you are working around the bug.

> Your point is correct...
> I would suggest using tst_syscall() to check for syscall undefined instead

Well, I guess we don't want to use tst_syscall() otherwise it would call
tst_brk(). I proposed similar patch some time ago [1], I suppose you told me
privately exactly this.

[1] https://patchwork.ozlabs.org/project/ltp/patch/20240124142108.303782-1-pvorel@suse.cz/

> of this modification. How about this modification? 

> ```
> --- a/lib/tst_fd.c
> +++ b/lib/tst_fd.c
> @@ -255,7 +255,8 @@ static void open_memfd(struct tst_fd *fd)

>  static void open_memfd_secret(struct tst_fd *fd)
>  {
> -       fd->fd = syscall(__NR_memfd_secret, 0);
> +       fd->fd = tst_syscall(__NR_memfd_secret, 0);
>         if (fd->fd < 0) {
>                 tst_res(TCONF | TERRNO,
>                         "Skipping %s", tst_fd_desc(fd));


Therefore how about this?

	if ((tst_kvercmp(5, 14, 0)) < 0) {
		tst_res(TCONF, "accept() on %s: skipping due old kernel", tst_fd_desc(fd));
		return;
	}

Kind regards,
Petr

> Best regards,
>   Nobuhiro

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[nobuhiro1.iwamatsu]

Hi!

Thanks for your comment.

> > > > $ ./testcases/kernel/syscalls/accept/accept03
> > > > tst_test.c:1733: TINFO: LTP version: 20240524
> > > > tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
> > > > accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
> > > > accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
> > > > tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
> > > > accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
> > > > tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
> > > > accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
> > > > accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
> > > > accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
> > > > tst_test.c:1677: TBROK: Test killed by SIGILL!
> 
> > > This looks like a bug either in kernel or libc.
> 
> > This is caused by __NR_memfd_secure being defined as -1 (0xffffffff)and
> "Illegal instruction"
> > occurs when syscall() is executed. And this problem does not occur on
> x86_64.
> > I cannot decide if this is a bug or not. I can't decide if this is a
> > bug or not, because this behavior has existed for a long time.
> 
> Interesting. But it'd be good to discuss it, right? In case there is something to
> improve. Cc linux-arm-kernel ML.

Indeed, Thank you.

> 
> > > > Summary:
> > > > passed   20
> > > > failed   0
> > > > broken   1
> > > > skipped  2
> > > > warnings 0
> > > > ```
> 
> > > > Closed: #1145
> > > > Signed-off-by: Nobuhiro Iwamatsu
> > > > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > > ---
> > > >  lib/tst_fd.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> 
> > > > diff --git a/lib/tst_fd.c b/lib/tst_fd.c index
> > > > 6538a098c..53f583fa0
> > > > 100644
> > > > --- a/lib/tst_fd.c
> > > > +++ b/lib/tst_fd.c
> > > > @@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)
> 
> > > >  static void open_memfd_secret(struct tst_fd *fd)  {
> > > > +	if ((tst_kvercmp(5, 14, 0)) < 0) {
> > > > +		tst_res(TINFO, "accept() on %s: Linux kernel version
> is before
> > > than v5.14", tst_fd_desc(fd));
> 
> 
> > > > +		errno = ENOSYS;
> > > > +		goto skip;
> > > > +	}
> > > > +
> > > >  	fd->fd = syscall(__NR_memfd_secret, 0);
> > > > +
> > > >  	if (fd->fd < 0) {
> > > > +skip:
> > > >  		tst_res(TCONF | TERRNO,
> > > >  			"Skipping %s", tst_fd_desc(fd));
> > > >  	}
> 
> > > And this looks like you are working around the bug.
> 
> > Your point is correct...
> > I would suggest using tst_syscall() to check for syscall undefined
> > instead
> 
> Well, I guess we don't want to use tst_syscall() otherwise it would call tst_brk().
> I proposed similar patch some time ago [1], I suppose you told me privately
> exactly this.
> 
> [1]
> https://patchwork.ozlabs.org/project/ltp/patch/20240124142108.303782-1-p
> vorel@suse.cz/]

I see, I understand.

> 
> > of this modification. How about this modification?
> 
> > ```
> > --- a/lib/tst_fd.c
> > +++ b/lib/tst_fd.c
> > @@ -255,7 +255,8 @@ static void open_memfd(struct tst_fd *fd)
> 
> >  static void open_memfd_secret(struct tst_fd *fd)  {
> > -       fd->fd = syscall(__NR_memfd_secret, 0);
> > +       fd->fd = tst_syscall(__NR_memfd_secret, 0);
> >         if (fd->fd < 0) {
> >                 tst_res(TCONF | TERRNO,
> >                         "Skipping %s", tst_fd_desc(fd));
> 
> 
> Therefore how about this?
> 
> 	if ((tst_kvercmp(5, 14, 0)) < 0) {
> 		tst_res(TCONF, "accept() on %s: skipping due old kernel",
> tst_fd_desc(fd));
> 		return;
> 	}
> 

I did not explain well enough.
The memfd_secret syscall itself is supported in 5.14, but is implemented on i386, x86_64, s390, and s390x with latest kernel.
Other architectures are not supported. The above patch causes the same problem with the latest kernel.
So, I create with the following patch based on your comments. How about this?

--- a/lib/tst_fd.c
+++ b/lib/tst_fd.c
@@ -255,11 +255,20 @@ static void open_memfd(struct tst_fd *fd)

 static void open_memfd_secret(struct tst_fd *fd)
 {
+#if defined(__i386__) || defined(__x86_64__) || defined(__s390__) || defined(__s390x__)
+       if ((tst_kvercmp(5, 14, 0)) < 0) {
+               tst_res(TCONF, "%s: skipping due old kernel", tst_fd_desc(fd));
+               return;
+       }
+
        fd->fd = syscall(__NR_memfd_secret, 0);
        if (fd->fd < 0) {
                tst_res(TCONF | TERRNO,
                        "Skipping %s", tst_fd_desc(fd));
        }
+#else
+       tst_res(TCONF, "%s not supported on this architecture", tst_fd_desc(fd));
+#endif
 }

 static struct tst_fd_desc fd_desc[] = {
@@ -287,7 +296,7 @@ static struct tst_fd_desc fd_desc[] = {
        [TST_FD_FSPICK] = {.open_fd = open_fspick, .desc = "fspick"},
        [TST_FD_OPEN_TREE] = {.open_fd = open_open_tree, .desc = "open_tree"},
        [TST_FD_MEMFD] = {.open_fd = open_memfd, .desc = "memfd"},
-       [TST_FD_MEMFD_SECRET] = {.open_fd = open_memfd_secret, .desc = "memfd secret"},
+       [TST_FD_MEMFD_SECRET] = {.open_fd = open_memfd_secret, .desc = "memfd_secret"},

Best regards,
  Nobuhiro

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Petr Vorel]

> Hi!

> Thanks for your comment.

> > > > > $ ./testcases/kernel/syscalls/accept/accept03
> > > > > tst_test.c:1733: TINFO: LTP version: 20240524
> > > > > tst_test.c:1617: TINFO: Timeout per run is 0h 00m 30s
> > > > > accept03.c:58: TPASS: accept() on file : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on O_PATH file : EBADF (9)
> > > > > accept03.c:58: TPASS: accept() on directory : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on /dev/zero : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on /proc/self/maps : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on pipe read end : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on pipe write end : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on epoll : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on eventfd : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on signalfd : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on timerfd : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on pidfd : ENOTSOCK (88)
> > > > > tst_fd.c:151: TCONF: Skipping fanotify: ENOSYS (38)
> > > > > accept03.c:58: TPASS: accept() on inotify : ENOTSOCK (88)
> > > > > tst_fd.c:170: TCONF: Skipping userfaultfd: ENOSYS (38)
> > > > > accept03.c:58: TPASS: accept() on perf event : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on io uring : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on bpf map : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on fsopen : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on fspick : ENOTSOCK (88)
> > > > > accept03.c:58: TPASS: accept() on open_tree : EBADF (9)
> > > > > accept03.c:58: TPASS: accept() on memfd : ENOTSOCK (88)
> > > > > tst_test.c:1677: TBROK: Test killed by SIGILL!

> > > > This looks like a bug either in kernel or libc.

> > > This is caused by __NR_memfd_secure being defined as -1 (0xffffffff)and
> > "Illegal instruction"
> > > occurs when syscall() is executed. And this problem does not occur on
> > x86_64.
> > > I cannot decide if this is a bug or not. I can't decide if this is a
> > > bug or not, because this behavior has existed for a long time.

> > Interesting. But it'd be good to discuss it, right? In case there is something to
> > improve. Cc linux-arm-kernel ML.

> Indeed, Thank you.


> > > > > Summary:
> > > > > passed   20
> > > > > failed   0
> > > > > broken   1
> > > > > skipped  2
> > > > > warnings 0
> > > > > ```

> > > > > Closed: #1145
> > > > > Signed-off-by: Nobuhiro Iwamatsu
> > > > > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > > > ---
> > > > >  lib/tst_fd.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)

> > > > > diff --git a/lib/tst_fd.c b/lib/tst_fd.c index
> > > > > 6538a098c..53f583fa0
> > > > > 100644
> > > > > --- a/lib/tst_fd.c
> > > > > +++ b/lib/tst_fd.c
> > > > > @@ -255,8 +255,16 @@ static void open_memfd(struct tst_fd *fd)

> > > > >  static void open_memfd_secret(struct tst_fd *fd)  {
> > > > > +	if ((tst_kvercmp(5, 14, 0)) < 0) {
> > > > > +		tst_res(TINFO, "accept() on %s: Linux kernel version
> > is before
> > > > than v5.14", tst_fd_desc(fd));


> > > > > +		errno = ENOSYS;
> > > > > +		goto skip;
> > > > > +	}
> > > > > +
> > > > >  	fd->fd = syscall(__NR_memfd_secret, 0);
> > > > > +
> > > > >  	if (fd->fd < 0) {
> > > > > +skip:
> > > > >  		tst_res(TCONF | TERRNO,
> > > > >  			"Skipping %s", tst_fd_desc(fd));
> > > > >  	}

> > > > And this looks like you are working around the bug.

> > > Your point is correct...
> > > I would suggest using tst_syscall() to check for syscall undefined
> > > instead

> > Well, I guess we don't want to use tst_syscall() otherwise it would call tst_brk().
> > I proposed similar patch some time ago [1], I suppose you told me privately
> > exactly this.

> > [1]
> > https://patchwork.ozlabs.org/project/ltp/patch/20240124142108.303782-1-p
> > vorel@suse.cz/]

> I see, I understand.


> > > of this modification. How about this modification?

> > > ```
> > > --- a/lib/tst_fd.c
> > > +++ b/lib/tst_fd.c
> > > @@ -255,7 +255,8 @@ static void open_memfd(struct tst_fd *fd)

> > >  static void open_memfd_secret(struct tst_fd *fd)  {
> > > -       fd->fd = syscall(__NR_memfd_secret, 0);
> > > +       fd->fd = tst_syscall(__NR_memfd_secret, 0);
> > >         if (fd->fd < 0) {
> > >                 tst_res(TCONF | TERRNO,
> > >                         "Skipping %s", tst_fd_desc(fd));


> > Therefore how about this?

> > 	if ((tst_kvercmp(5, 14, 0)) < 0) {
> > 		tst_res(TCONF, "accept() on %s: skipping due old kernel",
> > tst_fd_desc(fd));
> > 		return;
> > 	}


> I did not explain well enough.
> The memfd_secret syscall itself is supported in 5.14, but is implemented on i386, x86_64, s390, and s390x with latest kernel.
> Other architectures are not supported. The above patch causes the same problem with the latest kernel.
> So, I create with the following patch based on your comments. How about this?

> --- a/lib/tst_fd.c
> +++ b/lib/tst_fd.c
> @@ -255,11 +255,20 @@ static void open_memfd(struct tst_fd *fd)

>  static void open_memfd_secret(struct tst_fd *fd)
>  {
> +#if defined(__i386__) || defined(__x86_64__) || defined(__s390__) || defined(__s390x__)

Well, I would prefer to detect syscall support via ENOSYS (or whichever errno it
uses on the archs which does not implement it), otherwise it will always TCONF
regardless some arch may get support in the future. E.g. use the same approach
as tst_syscall(), but just use tst_res() instead of tst_brk() (first run the
syscall and then do the check - expect that errno != ENOSYS on these 4 archs on
kernel >= 5.14.

FYI We have usually this approach: we try to explicitly check whether certain
functionality works if arch or filesystem dependent (e.g. some arch supports
something), but sure still keep in mind that particular support can be extended
(to other archs or filesystems).

Also according to 1507f51255c9f ("mm: introduce memfd_secret system call to
create "secret" memory areas") [1] it's a configurable option (hidden via
CONFIG_EXPERT, but it can be disabled). Therefore the functionality can be
detected via check for CONFIG_SECRETMEM:

tst_kconfig_get("CONFIG_SECRETMEM") == 'y';

Therefore something like this?

static void open_memfd_secret(struct tst_fd *fd)
{
	fd->fd = syscall(__NR_memfd_secret, 0);

	if (errno == ENOSYS) {
		if (tst_kconfig_get("CONFIG_SECRETMEM") == 'y') {
			tst_brk(TBROK | TERRNO, "Broken memfd_secret() functionality");
		} else {
			tst_res(TCONF | TERRNO, "Skipping %s due missing memfd_secret()",
				tst_fd_desc(fd));
			return;
		}
	}

	if (fd->fd < 0)
		tst_res(TCONF | TERRNO, "Skipping %s", tst_fd_desc(fd));
}

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1507f51255c9ff07d75909a84e7c0d7f3c4b2f49

Therefore the original approach looked better to me.

Kind regards,
Petr

> +       if ((tst_kvercmp(5, 14, 0)) < 0) {
> +               tst_res(TCONF, "%s: skipping due old kernel", tst_fd_desc(fd));
> +               return;
> +       }
> +
>         fd->fd = syscall(__NR_memfd_secret, 0);
>         if (fd->fd < 0) {
>                 tst_res(TCONF | TERRNO,
>                         "Skipping %s", tst_fd_desc(fd));
>         }
> +#else
> +       tst_res(TCONF, "%s not supported on this architecture", tst_fd_desc(fd));
> +#endif
>  }

>  static struct tst_fd_desc fd_desc[] = {
> @@ -287,7 +296,7 @@ static struct tst_fd_desc fd_desc[] = {
>         [TST_FD_FSPICK] = {.open_fd = open_fspick, .desc = "fspick"},
>         [TST_FD_OPEN_TREE] = {.open_fd = open_open_tree, .desc = "open_tree"},
>         [TST_FD_MEMFD] = {.open_fd = open_memfd, .desc = "memfd"},
> -       [TST_FD_MEMFD_SECRET] = {.open_fd = open_memfd_secret, .desc = "memfd secret"},
> +       [TST_FD_MEMFD_SECRET] = {.open_fd = open_memfd_secret, .desc = "memfd_secret"},

> Best regards,
>   Nobuhiro

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Cyril Hrubis]

Hi!
> I would suggest using tst_syscall() to check for syscall undefined instead
> of this modification. How about this modification? 
> 
> ```
> --- a/lib/tst_fd.c
> +++ b/lib/tst_fd.c
> @@ -255,7 +255,8 @@ static void open_memfd(struct tst_fd *fd)
> 
>  static void open_memfd_secret(struct tst_fd *fd)
>  {
> -       fd->fd = syscall(__NR_memfd_secret, 0);
> +       fd->fd = tst_syscall(__NR_memfd_secret, 0);
>         if (fd->fd < 0) {
>                 tst_res(TCONF | TERRNO,
>                         "Skipping %s", tst_fd_desc(fd));
> ```

We cannot use tst_syscall() in the tst_fd library because it calls
tst_brk() with TCONF which exits the test immediately, but in this case
we actually want to continue with the rest of the tests.

I guess that the best fix is to add fallback definitions for
memfd_secret into include/lapi/syscalls/*.in files. That way we should
get -1 and EINVAL properly even when kernel does not support the
syscall.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Cyril Hrubis]

Hi!
Can we first try the simple solution by adding fallback definitions
before we add such complexity there?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Petr Vorel]

Hi Nobuhiro,

> memfd_secret is a syscall added since 5.14. On earlier kernels, tests such
> as accept03, readahead01 and splice07 that use memfd_secret fail.
> This adds a kernel version check to the tst_fd library when running tests using
> memfd_secret.

Cyril's suggestion to add fallback definitions sounds to me as a good idea:

https://lore.kernel.org/ltp/ZnVCcU6jOU98DYek@yuki/

	I guess that the best fix is to add fallback definitions for
	memfd_secret into include/lapi/syscalls/*.in files. That way we should
	get -1 and EINVAL properly even when kernel does not support the
	syscall.

Could you please try that first?

Kind regards,
Petr

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Petr Vorel]

Hi Nobuhiro,

Cyril's suggestion to add fallback definitions sounds to me as a good idea:

https://lore.kernel.org/ltp/ZnVCcU6jOU98DYek@yuki/

	I guess that the best fix is to add fallback definitions for
	memfd_secret into include/lapi/syscalls/*.in files. That way we should
	get -1 and EINVAL properly even when kernel does not support the
	syscall.

Could you please try that first?

Kind regards,
Petr

PS: I'm sorry for the noise, posted previously to the old version, which I
superseded in patchwork.

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[nobuhiro1.iwamatsu]

Hi Petr,

> -----Original Message-----
> From: Petr Vorel <pvorel@suse.cz>
> Sent: Friday, June 21, 2024 8:09 PM
> To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
> <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: ltp@lists.linux.it; Cyril Hrubis <chrubis@suse.cz>
> Subject: Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to
> memfd_secret
> 
> Hi Nobuhiro,
> 
> > memfd_secret is a syscall added since 5.14. On earlier kernels, tests
> > such as accept03, readahead01 and splice07 that use memfd_secret fail.
> > This adds a kernel version check to the tst_fd library when running
> > tests using memfd_secret.
> 
> Cyril's suggestion to add fallback definitions sounds to me as a good idea:
> 
> https://lore.kernel.org/ltp/ZnVCcU6jOU98DYek@yuki/
> 
> 	I guess that the best fix is to add fallback definitions for
> 	memfd_secret into include/lapi/syscalls/*.in files. That way we
> should
> 	get -1 and EINVAL properly even when kernel does not support the
> 	syscall.
> 
> Could you please try that first?

OK, I will create a v2 patch based on that suggestion.

> 
> Kind regards,
> Petr

Best regards,
  Nobuhiro


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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[Edward Liaw via ltp]

Hi Nobuhiro,

On Fri, Jun 28, 2024 at 1:40 AM <nobuhiro1.iwamatsu@toshiba.co.jp> wrote:
>
> Hi Petr,
>
> > -----Original Message-----
> > From: Petr Vorel <pvorel@suse.cz>
> > Sent: Friday, June 21, 2024 8:09 PM
> > To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
> > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Cc: ltp@lists.linux.it; Cyril Hrubis <chrubis@suse.cz>
> > Subject: Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to
> > memfd_secret
> >
> > Hi Nobuhiro,
> >
> > > memfd_secret is a syscall added since 5.14. On earlier kernels, tests
> > > such as accept03, readahead01 and splice07 that use memfd_secret fail.
> > > This adds a kernel version check to the tst_fd library when running
> > > tests using memfd_secret.
> >
> > Cyril's suggestion to add fallback definitions sounds to me as a good idea:
> >
> > https://lore.kernel.org/ltp/ZnVCcU6jOU98DYek@yuki/
> >
> >       I guess that the best fix is to add fallback definitions for
> >       memfd_secret into include/lapi/syscalls/*.in files. That way we
> > should
> >       get -1 and EINVAL properly even when kernel does not support the
> >       syscall.
> >
> > Could you please try that first?
>
> OK, I will create a v2 patch based on that suggestion.

A vendor on arm32 ran into this issueI and I didn't see a v2 submitted
yet, so I sent https://lore.kernel.org/ltp/20240711004220.1338086-2-edliaw@google.com/
which is working for me; do you mind testing it?.

>
> >
> > Kind regards,
> > Petr
>
> Best regards,
>   Nobuhiro
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp

Thanks,
Edward

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

Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret

[nobuhiro1.iwamatsu]

Hi Edward,

Sorry for the late reply. And thanks for your update!

> -----Original Message-----
> From: Edward Liaw <edliaw@google.com>
> Sent: Saturday, July 13, 2024 3:41 AM
> To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ)
> <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: pvorel@suse.cz; ltp@lists.linux.it
> Subject: Re: [LTP] [PATCH] lib: tst_fd: Add kernel version check to
> memfd_secret
> 

> >
> > OK, I will create a v2 patch based on that suggestion.
> 
> A vendor on arm32 ran into this issueI and I didn't see a v2 submitted yet, so I
> sent
> https://lore.kernel.org/ltp/20240711004220.1338086-2-edliaw@google.com/
> which is working for me; do you mind testing it?.

Yes, I have confirmed that this patch fixes the problem.
Thank you very much!

Best regards,
  Nobuhiro

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