Eduardo Otubo <ot...@linux.vnet.ibm.com> writes:
> The new 'trap' (debug) mode will capture the illegal system call before it is
> executed. The feature and the implementation is based on Will Drewry's
> patch - https://lkml.org/lkml/2012/4/12/449
>
> v4:
> * New files in v4
> * If SCMP_ACT_TRAP flag used when calling seccomp_init(), the kernel will
> send a SIGSYS every time a not whitelisted syscall is called. This
> sighandler install_seccomp_syscall_debug() is installed in this mode so
> we can intercept the signal and print to the user the illegal syscall.
> The process resumes after that.
> * The behavior of the code inside a signal handler sometimes is
> unpredictable (as stated in man 7 signals). That's why I deliberately
> used write() and _exit() functions, and had the string-to-int helper
> functions as well.
>
> Signed-off-by: Eduardo Otubo <ot...@linux.vnet.ibm.com>
> ---
> qemu-seccomp-debug.c | 95
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-seccomp-debug.h | 38 ++++++++++++++++++++
> 2 files changed, 133 insertions(+), 0 deletions(-)
> create mode 100644 qemu-seccomp-debug.c
> create mode 100644 qemu-seccomp-debug.h
>
> diff --git a/qemu-seccomp-debug.c b/qemu-seccomp-debug.c
> new file mode 100644
> index 0000000..162c2f1
> --- /dev/null
> +++ b/qemu-seccomp-debug.c
> @@ -0,0 +1,95 @@
> +
> +/*
> + * QEMU seccomp mode 2 support with libseccomp
> + * Debug system calls helper functions
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Eduardo Otubo <eot...@br.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#include "qemu-seccomp-debug.h"
> +#include "asm-generic/unistd.h"
This looks like an odd include to me. I assume you're relying on Linux
headers being installed? You should at least do <asm-generic/unistd.h>
but I wonder why you need this in the first place.
> +
> +#define safe_warn(data) write(STDERR_FILENO, (const void *) data,
> sizeof(data))
> +
> +static int count_digits(int number)
> +{
> + int digits = 0;
> + while (number) {
> + number /= 10;
> + digits++;
> + }
> +
> + return digits;
> +}
> +
> +static char *sput_i(int integer, char *string)
> +{
> + if (integer / 10 != 0) {
> + string = sput_i(integer / 10, string);
> + }
> + *string++ = (char) ('0' + integer % 10);
> + return string;
> +}
> +
> +static void int_to_asc(int integer, char *string)
> +{
> + *sput_i(integer, string) = '\n';
> +}
> +
> +static void syscall_debug(int nr, siginfo_t *info, void *void_context)
> +{
> + ucontext_t *ctx = (ucontext_t *) (void_context);
> + char errormsg[] = "seccomp: illegal syscall trapped: ";
> + char syscall_char[count_digits(__NR_syscalls) + 1];
> + int syscall_num = 0;
> +
> + if (info->si_code != SYS_SECCOMP) {
> + return;
> + }
> + if (!ctx) {
> + return;
> + }
> + syscall_num = ctx->uc_mcontext.gregs[REG_SYSCALL];
> + if (syscall_num < 0 || syscall_num >= __NR_syscalls) {
> + if ((safe_warn("seccomp: error reading syscall from register\n") <
> 0)) {
> + return;
> + }
> + return;
> + }
> + int_to_asc(syscall_num, syscall_char);
I assume you're doign this because of fear of signal safety? Is there a
reason to believe that snprintf() wouldn't be signal safe? Even if it's
not on the white list, the implementation can't reasonably rely on
global data, can it?
> + if ((safe_warn(errormsg) < 0) || (safe_warn(syscall_char) < 0)) {
> + return;
> + }
> + return;
> +}
> +
> +int install_seccomp_syscall_debug(void)
> +{
> + struct sigaction act;
> + sigset_t mask;
> +
> + memset(&act, 0, sizeof(act));
> + sigemptyset(&mask);
> + sigaddset(&mask, SIGSYS);
> +
> + act.sa_sigaction = &syscall_debug;
> + act.sa_flags = SA_SIGINFO;
> + if (sigaction(SIGSYS, &act, NULL) < 0) {
> + perror("seccomp: sigaction returned with errors\n");
> + return -1;
> + }
> + if (pthread_sigmask(SIG_UNBLOCK, &mask, NULL)) {
> + perror("seccomp: sigprocmask returned with errors\n");
> + return -1;
> + }
This looks fishy to me. We aggressively modify our signal mask in order
to launch a KVM VCPU so I'm pretty sure we'll quickly block SIGSYS. I
think you need to touch more code than this for it to work.
Regards,
Anthony Liguori
> + return 0;
> +}
> diff --git a/qemu-seccomp-debug.h b/qemu-seccomp-debug.h
> new file mode 100644
> index 0000000..d3863d6
> --- /dev/null
> +++ b/qemu-seccomp-debug.h
> @@ -0,0 +1,38 @@
> +/*
> + * QEMU seccomp mode 2 support with libseccomp
> + * Trap system calls helper functions
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Eduardo Otubo <eot...@br.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
Version 2 or later for all new files. Don't include this disclaimer in
new code.
Regards,
Anthony Liguori
> + */
> +#ifndef QEMU_SECCOMP_TRAP_H
> +#define QEMU_SECCOMP_TRAP_H
> +
> +#include <signal.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +#if defined(__i386__)
> +#define REG_SYSCALL REG_EAX
> +#elif defined(__x86_64__)
> +#define REG_SYSCALL REG_RAX
> +#else
> +#error Unsupported platform
> +#endif
> +
> +#ifndef SYS_SECCOMP
> +#define SYS_SECCOMP 1
> +#endif
> +
> +int install_seccomp_syscall_debug(void);
> +
> +#endif
> --
> 1.7.1
