Messages in this thread | | | Date | Mon, 26 Mar 2012 08:56:39 -0700 | Subject | Re: [PATCH] audit: always report seccomp violations | From | Kees Cook <> |
| |
On Sun, Mar 25, 2012 at 11:47 AM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 3/23/2012 4:32 PM, Kees Cook wrote: >> When a program violates its own seccomp rules, that is a pretty dire >> situation, and the audit message should always be reported (not just >> when there is already a rule active for the process). > > Hmm. If the program is never going to violate its own > seccomp rules it seems sort of silly to have them in the > first place, doesn't it? Oh, I know that the expectation > of seccomp is that the application would only try something > you've disallowed if it gets compromised. Problem is that
Well, either compromised or doing something new (e.g. a library in the code has changed).
> Modern Programmers tend to rely very heavily on the opaque > behavior of APIs that they don't understand nor particularly > care if they understand. When assumptions are made about the > behavior of the API code, and the API code changes, as > occurs with amazing frequency on today's mobile devices, > there are going to be surprises. I would wager that the > modern frequency of API changes will result in this behavior > being very unpopular.
You seem to be advocating for my patch -- instead of the program "silently" getting killed, now there will be notification. A seccomp failure is extremely uncommon; much less common that core dumps. This is why it should always be reported -- it is uncommon and important to notice.
>> This change makes the audit_seccomp() logic similar to audit_core_dumps() >> (it does not require an active context). Since core dumps are more >> common, they sit behind an "audit_enabled" test. Audit reports of seccomp >> failures should always be visible, and fall back to printk when auditd >> is not running. >> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> include/linux/audit.h | 8 +------- >> kernel/auditsc.c | 11 +++++++++-- >> 2 files changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/audit.h b/include/linux/audit.h >> index ed3ef19..596077f 100644 >> --- a/include/linux/audit.h >> +++ b/include/linux/audit.h >> @@ -463,7 +463,7 @@ extern void audit_putname(const char *name); >> extern void __audit_inode(const char *name, const struct dentry *dentry); >> extern void __audit_inode_child(const struct dentry *dentry, >> const struct inode *parent); >> -extern void __audit_seccomp(unsigned long syscall); >> +extern void audit_seccomp(unsigned long syscall); >> extern void __audit_ptrace(struct task_struct *t); >> >> static inline int audit_dummy_context(void) >> @@ -508,12 +508,6 @@ static inline void audit_inode_child(const struct dentry *dentry, >> } >> void audit_core_dumps(long signr); >> >> -static inline void audit_seccomp(unsigned long syscall) >> -{ >> - if (unlikely(!audit_dummy_context())) >> - __audit_seccomp(syscall); >> -} >> - >> static inline void audit_ptrace(struct task_struct *t) >> { >> if (unlikely(!audit_dummy_context())) >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index af1de0f..a5caecd 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -2693,7 +2693,7 @@ static void audit_log_abend(struct audit_buffer *ab, char *reason, long signr) >> * @signr: signal value >> * >> * If a process ends with a core dump, something fishy is going on and we >> - * should record the event for investigation. >> + * should record the event for investigation, if auditing is enabled. >> */ >> void audit_core_dumps(long signr) >> { >> @@ -2710,7 +2710,14 @@ void audit_core_dumps(long signr) >> audit_log_end(ab); >> } >> >> -void __audit_seccomp(unsigned long syscall) >> +/** >> + * audit_seccomp - record information about processes that violate seccomp >> + * @syscall: syscall number that triggered the seccomp violation >> + * >> + * If a process violates its own seccomp rules, something has gone very >> + * wrong, and this event should always be reported for investigation. >> + */ >> +void audit_seccomp(unsigned long syscall) >> { >> struct audit_buffer *ab; >> >
-- Kees Cook ChromeOS Security -- 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/
| |