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

[nobuhiro1.iwamatsu@toshiba.co.jp]

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