[LTP] [PATCH] lib: tst_fd: Add kernel version check to memfd_secret
Petr Vorel
pvorel@suse.cz
Thu Jun 20 15:25:11 CEST 2024
> 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
More information about the ltp
mailing list