Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932130Ab2BVTsP (ORCPT ); Wed, 22 Feb 2012 14:48:15 -0500 Received: from mail-lpp01m010-f46.google.com ([209.85.215.46]:64252 "EHLO mail-lpp01m010-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755479Ab2BVTsK convert rfc822-to-8bit (ORCPT ); Wed, 22 Feb 2012 14:48:10 -0500 MIME-Version: 1.0 In-Reply-To: <9edbabb2262e3d91a7b8c75dbec03d7f.squirrel@webmail.greenhost.nl> References: <1329845435-2313-1-git-send-email-wad@chromium.org> <1329845435-2313-7-git-send-email-wad@chromium.org> <9edbabb2262e3d91a7b8c75dbec03d7f.squirrel@webmail.greenhost.nl> Date: Wed, 22 Feb 2012 13:48:08 -0600 Message-ID: Subject: Re: [PATCH v10 07/11] signal, x86: add SIGSYS info and make it synchronous. From: Will Drewry To: Indan Zupancic Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, kernel-hardening@lists.openwall.com, netdev@vger.kernel.org, x86@kernel.org, arnd@arndb.de, davem@davemloft.net, hpa@zytor.com, mingo@redhat.com, oleg@redhat.com, peterz@infradead.org, rdunlap@xenotime.net, mcgrathr@chromium.org, tglx@linutronix.de, luto@mit.edu, eparis@redhat.com, serge.hallyn@canonical.com, djm@mindrot.org, scarybeasts@gmail.com, pmoore@redhat.com, akpm@linux-foundation.org, corbet@lwn.net, eric.dumazet@gmail.com, markus@chromium.org, keescook@chromium.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6669 Lines: 167 On Wed, Feb 22, 2012 at 2:34 AM, Indan Zupancic wrote: > On Tue, February 21, 2012 18:30, Will Drewry wrote: >> This change enables SIGSYS, defines _sigfields._sigsys, and adds >> x86 (compat) arch support. ?_sigsys defines fields which allow >> a signal handler to receive the triggering system call number, >> the relevant AUDIT_ARCH_* value for that number, and the address >> of the callsite. >> >> To ensure that SIGSYS delivery occurs on return from the triggering >> system call, SIGSYS is added to the SYNCHRONOUS_MASK macro. ?I'm >> this is enough to ensure it will be synchronous or if it is explicitly >> required to ensure an immediate delivery of the signal upon return from >> the blocked system call. >> >> The first consumer of SIGSYS would be seccomp filter. ?In particular, >> a filter program could specify a new return value, SECCOMP_RET_TRAP, >> which would result in the system call being denied and the calling >> thread signaled. ?This also means that implementing arch-specific >> support can be dependent upon HAVE_ARCH_SECCOMP_FILTER. > > I think others said this is useful, but I don't see how. Easier > debugging compared to checking return values? > > I suppose SIGSYS can be blocked, so there is no guarantee the process > will be killed. Yeah, this allows for in-process system call emulation, if desired, or for the process to dump core/etc. With RET_ERRNO or RET_KILL, there isn't any feedback to the system about the state of the process. Kill populates audit_seccomp and dmesg, but if the application user/developer isn't the system admin, installing audit bits or checking system logs seems onerous. >> v10: - first version based on suggestion >> >> Suggested-by: H. Peter Anvin >> Signed-off-by: Will Drewry >> --- >> ?arch/x86/ia32/ia32_signal.c ? | ? ?4 ++++ >> ?arch/x86/include/asm/ia32.h ? | ? ?6 ++++++ >> ?include/asm-generic/siginfo.h | ? 18 ++++++++++++++++++ >> ?kernel/signal.c ? ? ? ? ? ? ? | ? ?2 +- >> ?4 files changed, 29 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c >> index 6557769..c81d2c7 100644 >> --- a/arch/x86/ia32/ia32_signal.c >> +++ b/arch/x86/ia32/ia32_signal.c >> @@ -73,6 +73,10 @@ int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t >> *from) >> ? ? ? ? ? ? ? ? ? ? ? switch (from->si_code >> 16) { >> ? ? ? ? ? ? ? ? ? ? ? case __SI_FAULT >> 16: >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> + ? ? ? ? ? ? ? ? ? ? case __SI_SYS >> 16: >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? put_user_ex(from->si_syscall, &to->si_syscall); >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? put_user_ex(from->si_arch, &to->si_arch); >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> ? ? ? ? ? ? ? ? ? ? ? case __SI_CHLD >> 16: >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? put_user_ex(from->si_utime, &to->si_utime); >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? put_user_ex(from->si_stime, &to->si_stime); >> diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h >> index 1f7e625..541485f 100644 >> --- a/arch/x86/include/asm/ia32.h >> +++ b/arch/x86/include/asm/ia32.h >> @@ -126,6 +126,12 @@ typedef struct compat_siginfo { >> ? ? ? ? ? ? ? ? ? ? ? int _band; ? ? ?/* POLL_IN, POLL_OUT, POLL_MSG */ >> ? ? ? ? ? ? ? ? ? ? ? int _fd; >> ? ? ? ? ? ? ? } _sigpoll; >> + >> + ? ? ? ? ? ? struct { >> + ? ? ? ? ? ? ? ? ? ? unsigned int _call_addr; /* calling insn */ > > Why an int here, but a pointer below? This is the compat version and it expects to just use an unsigned int (see the _addr entry in _sigfault earlier in the same file). >> + ? ? ? ? ? ? ? ? ? ? int _syscall; ? /* triggering system call number */ >> + ? ? ? ? ? ? ? ? ? ? unsigned int _arch; ? ? /* AUDIT_ARCH_* of syscall */ >> + ? ? ? ? ? ? } _sigsys; >> ? ? ? } _sifields; >> ?} compat_siginfo_t; >> >> diff --git a/include/asm-generic/siginfo.h b/include/asm-generic/siginfo.h >> index 0dd4e87..a83b478 100644 >> --- a/include/asm-generic/siginfo.h >> +++ b/include/asm-generic/siginfo.h >> @@ -90,6 +90,13 @@ typedef struct siginfo { >> ? ? ? ? ? ? ? ? ? ? ? __ARCH_SI_BAND_T _band; /* POLL_IN, POLL_OUT, POLL_MSG */ >> ? ? ? ? ? ? ? ? ? ? ? int _fd; >> ? ? ? ? ? ? ? } _sigpoll; >> + >> + ? ? ? ? ? ? /* SIGSYS */ >> + ? ? ? ? ? ? struct { >> + ? ? ? ? ? ? ? ? ? ? void __user *_call_addr; /* calling insn */ > > Is this a user instruction pointer or a filter instruction? User instruction pointer, I'll clarify. >> + ? ? ? ? ? ? ? ? ? ? int _syscall; ? /* triggering system call number */ >> + ? ? ? ? ? ? ? ? ? ? unsigned int _arch; ? ? /* AUDIT_ARCH_* of syscall */ >> + ? ? ? ? ? ? } _sigsys; >> ? ? ? } _sifields; >> ?} siginfo_t; >> >> @@ -116,6 +123,9 @@ typedef struct siginfo { >> ?#define si_addr_lsb ?_sifields._sigfault._addr_lsb >> ?#define si_band ? ? ? ? ? ? ?_sifields._sigpoll._band >> ?#define si_fd ? ? ? ? ? ? ? ?_sifields._sigpoll._fd >> +#define si_call_addr _sifields._sigsys._call_addr >> +#define si_syscall ? _sifields._sigsys._syscall >> +#define si_arch ? ? ? ? ? ? ?_sifields._sigsys._arch >> >> ?#ifdef __KERNEL__ >> ?#define __SI_MASK ? ?0xffff0000u >> @@ -126,6 +136,7 @@ typedef struct siginfo { >> ?#define __SI_CHLD ? ?(4 << 16) >> ?#define __SI_RT ? ? ? ? ? ? ?(5 << 16) >> ?#define __SI_MESGQ ? (6 << 16) >> +#define __SI_SYS ? ? (7 << 16) >> ?#define __SI_CODE(T,N) ? ? ? ((T) | ((N) & 0xffff)) >> ?#else >> ?#define __SI_KILL ? ?0 >> @@ -135,6 +146,7 @@ typedef struct siginfo { >> ?#define __SI_CHLD ? ?0 >> ?#define __SI_RT ? ? ? ? ? ? ?0 >> ?#define __SI_MESGQ ? 0 >> +#define __SI_SYS ? ? 0 >> ?#define __SI_CODE(T,N) ? ? ? (N) >> ?#endif >> >> @@ -232,6 +244,12 @@ typedef struct siginfo { >> ?#define NSIGPOLL ? ? 6 >> >> ?/* >> + * SIGSYS si_codes >> + */ >> +#define SYS_SECCOMP ? ? ? ? ?(__SI_SYS|1) ? ?/* seccomp triggered */ >> +#define NSIGSYS ? ? ?1 >> + >> +/* >> ? * sigevent definitions >> ? * >> ? * It seems likely that SIGEV_THREAD will have to be handled from >> diff --git a/kernel/signal.c b/kernel/signal.c >> index c73c428..7573819 100644 >> --- a/kernel/signal.c >> +++ b/kernel/signal.c >> @@ -160,7 +160,7 @@ void recalc_sigpending(void) >> >> ?#define SYNCHRONOUS_MASK \ >> ? ? ? (sigmask(SIGSEGV) | sigmask(SIGBUS) | sigmask(SIGILL) | \ >> - ? ? ?sigmask(SIGTRAP) | sigmask(SIGFPE)) >> + ? ? ?sigmask(SIGTRAP) | sigmask(SIGFPE) | sigmask(SIGSYS)) >> >> ?int next_signal(struct sigpending *pending, sigset_t *mask) >> ?{ >> -- thanks! will -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/