Re: [LTP] [PATCH v1] seccomp01.c: Add SECCOMP_RET_USER_NOTIF check

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> This case will report EINVAL error when execute SAFE_IOCTL(notifyFd,
> SECCOMP_IOCTL_NOTIF_RECV, req) such as 5.6.19, so i put current case's
> .min_kver = "5.7.19"
>
> NOTE: If your old kernel compile env is ubuntu 22.04 LTS, better use
> old gcc-8 and also apply patch base following link:
> https://www.spinics.net/lists/kernel/msg3797871.html
>
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  configure.ac                                  |   1 +
>  include/lapi/seccomp.h                        |   7 +
>  runtest/syscalls                              |   2 +
>  testcases/kernel/syscalls/seccomp/.gitignore  |   1 +
>  testcases/kernel/syscalls/seccomp/Makefile    |   8 +
>  testcases/kernel/syscalls/seccomp/seccomp01.c | 456 ++++++++++++++++++
>  6 files changed, 475 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/seccomp/.gitignore
>  create mode 100644 testcases/kernel/syscalls/seccomp/Makefile
>  create mode 100644 testcases/kernel/syscalls/seccomp/seccomp01.c
>
> diff --git a/configure.ac b/configure.ac
> index 662c4c058..6cea35cb4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -138,6 +138,7 @@ AC_CHECK_FUNCS_ONCE([ \
>      renameat \
>      renameat2 \
>      sched_getcpu \
> +    seccomp \
>      sendmmsg \
>      sethostid \
>      setns \
> diff --git a/include/lapi/seccomp.h b/include/lapi/seccomp.h
> index 29819ba6f..cfb3da55d 100644
> --- a/include/lapi/seccomp.h
> +++ b/include/lapi/seccomp.h
> @@ -37,4 +37,11 @@ struct seccomp_data {
>  };
>  
>  #endif /* HAVE_LINUX_SECCOMP_H*/
> +
> +# ifndef HAVE_SECCOMP
> +int seccomp(unsigned int operation, unsigned int flags, void *args)
> +{
> +	return syscall(__NR_seccomp, operation, flags, args);
> +}
> +# endif /* HAVE_SECCOMP */
>  #endif /* LAPI_SECCOMP_H__ */
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 4f1ee1f34..544610d63 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1242,6 +1242,8 @@ select02 select02
>  select03 select03
>  select04 select04
>  
> +seccomp01 seccomp01
> +
>  semctl01 semctl01
>  semctl02 semctl02
>  semctl03 semctl03
> diff --git a/testcases/kernel/syscalls/seccomp/.gitignore b/testcases/kernel/syscalls/seccomp/.gitignore
> new file mode 100644
> index 000000000..9196906cf
> --- /dev/null
> +++ b/testcases/kernel/syscalls/seccomp/.gitignore
> @@ -0,0 +1 @@
> +seccomp01
> diff --git a/testcases/kernel/syscalls/seccomp/Makefile b/testcases/kernel/syscalls/seccomp/Makefile
> new file mode 100644
> index 000000000..49238eee0
> --- /dev/null
> +++ b/testcases/kernel/syscalls/seccomp/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +# Copyright (c) 2023 Wei Gao <wegao@suse.com>
> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/seccomp/seccomp01.c b/testcases/kernel/syscalls/seccomp/seccomp01.c
> new file mode 100644
> index 000000000..bf23fe8f7
> --- /dev/null
> +++ b/testcases/kernel/syscalls/seccomp/seccomp01.c
> @@ -0,0 +1,456 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023 Michael Kerrisk <mtk.manpages@gmail.com>
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify seccomp and seccomp_user_notif
> + */
> +
> +#define _GNU_SOURCE
> +#include <sys/types.h>
> +#include <sys/prctl.h>
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <signal.h>
> +#include <stddef.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <linux/audit.h>
> +#include <sys/syscall.h>
> +#include <sys/stat.h>
> +#include <linux/filter.h>
> +#include <linux/seccomp.h>
> +#include <sys/ioctl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +
> +#include "tst_test.h"
> +#include "lapi/seccomp.h"
> +
> +#define TMP_PREFIX_DIR "/tmp/ltp_test"
> +#define CWD_DIR "./abc"
> +#define OTHER_DIR "/aa"
> +
> +static struct tcase {
> +	char *dir;
> +	int expect_ret;
> +	char *desc;
> +} tcases[] = {
> +	{TMP_PREFIX_DIR, strlen(TMP_PREFIX_DIR), "pathname begins with the prefix /tmp/"},
> +	{CWD_DIR, 0,  "pathname begins with ./"},
> +	{OTHER_DIR, -1, "pathname begins with /abc"},
> +};
> +
> +static int sendfd(int sockfd, int fd)
> +{
> +	struct msghdr msgh;
> +	struct iovec iov;
> +	int data;
> +	struct cmsghdr *cmsgp;
> +
> +	/* Allocate a char array of suitable size to hold the ancillary data.
> +	 * However, since this buffer is in reality a 'struct cmsghdr', use a
> +	 * union to ensure that it is suitable aligned.
> +	 */

Comments like this are not LTP style. Explanations can go in the
description. Inline comments are reserved for special cases.

> +	union {
> +		char   buf[CMSG_SPACE(sizeof(int))];
> +		/* Space large enough to hold an 'int' */
> +		struct cmsghdr align;
> +	} controlMsg;

Also controlMsg is not LTP style. This continues thoughout the patch.

-- 
Thank you,
Richard.

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