[PATCH 0/2] Syscall auditing lite

[PATCH 0/2] Syscall auditing lite

[Andy Lutomirski]

I've made no secret of the fact that I dislike syscall auditing.  As far
as I can tell, the main technical (i.e. not compliance-related) use of
syscall auditing is to supply some useful context information to go
along with events like AVC denials.

CONFIG_AUDITSYSCALL is serious overkill to do this.  kernel/auditsc.c is
~2500 lines of terror.

This patchset accomplishes the same goal, more usefully, with no
overhead at all, in under 70 lines of code.  It tries to coexist cleanly
with CONFIG_AUDITSYSCALL.

This is only implemented for x86.  Other architectures can add support
fairly easily, I think.

Andy Lutomirski (2):
  x86,syscall: Add syscall_in_syscall to test whether we're in a syscall
  audit: Syscall auditing lite

 arch/x86/Kconfig               |  1 +
 arch/x86/include/asm/syscall.h | 21 ++++++++++++++++++++
 init/Kconfig                   |  3 +++
 kernel/audit.c                 | 44 +++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 68 insertions(+), 1 deletion(-)

-- 
1.9.3

[PATCH 1/2] x86,syscall: Add syscall_in_syscall to test whether we're in a syscall

[Andy Lutomirski]

syscall_in_syscall will return true if we're in a real syscall and
will return false if we're not in a syscall.  If we're in a bad
syscall, the return value can vary.

The idea is to use this to come up with a much simpler replacement
for syscall auditing.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig               |  1 +
 arch/x86/include/asm/syscall.h | 21 +++++++++++++++++++++
 init/Kconfig                   |  3 +++
 3 files changed, 25 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 25d2c6f..e2602d4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -130,6 +130,7 @@ config X86
 	select HAVE_CC_STACKPROTECTOR
 	select GENERIC_CPU_AUTOPROBE
 	select HAVE_ARCH_AUDITSYSCALL
+	select HAVE_SYSCALL_IN_SYSCALL
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
index d6a756a..91e38b3 100644
--- a/arch/x86/include/asm/syscall.h
+++ b/arch/x86/include/asm/syscall.h
@@ -23,6 +23,27 @@
 typedef void (*sys_call_ptr_t)(void);
 extern const sys_call_ptr_t sys_call_table[];
 
+/**
+ * syscall_in_syscall() - are we in a syscall context?
+ * @task: The task to query.
+ * @regs: The task's pt_regs.
+ *
+ * This checks whether we are in a syscall.  If it returns true, then
+ * syscall_get_nr(), etc are usable and the current task is guaranteed
+ * to either die or to go through the syscall exit path when the syscall
+ * is done.
+ *
+ * If it returns false, no particular guarantees are made.  In
+ * particular, a malicious task can issue a syscall that causes
+ * syscall_in_syscall to return false.  Such a syscall won't do much,
+ * but it can still cause tracing code and such to run.
+ */
+static inline bool syscall_in_syscall(struct task_struct *task,
+				      struct pt_regs *regs)
+{
+	return regs->orig_ax != -1;
+}
+
 /*
  * Only the low 32 bits of orig_ax are meaningful, so we return int.
  * This importantly ignores the high bits on 64-bit, so comparisons
diff --git a/init/Kconfig b/init/Kconfig
index 9d3585b..bad2053 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -295,6 +295,9 @@ config AUDIT
 config HAVE_ARCH_AUDITSYSCALL
 	bool
 
+config HAVE_SYSCALL_IN_SYSCALL
+	bool
+
 config AUDITSYSCALL
 	bool "Enable system-call auditing support"
 	depends on AUDIT && HAVE_ARCH_AUDITSYSCALL
-- 
1.9.3

Re: [PATCH 1/2] x86,syscall: Add syscall_in_syscall to test whether we're in a syscall

[Andy Lutomirski]

On May 30, 2014 2:58 PM, "Andy Lutomirski" <luto@amacapital.net> wrote:
>
> syscall_in_syscall will return true if we're in a real syscall and
> will return false if we're not in a syscall.  If we're in a bad
> syscall, the return value can vary.
>
> The idea is to use this to come up with a much simpler replacement
> for syscall auditing.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/Kconfig               |  1 +
>  arch/x86/include/asm/syscall.h | 21 +++++++++++++++++++++
>  init/Kconfig                   |  3 +++
>  3 files changed, 25 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 25d2c6f..e2602d4 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -130,6 +130,7 @@ config X86
>         select HAVE_CC_STACKPROTECTOR
>         select GENERIC_CPU_AUTOPROBE
>         select HAVE_ARCH_AUDITSYSCALL
> +       select HAVE_SYSCALL_IN_SYSCALL
>
>  config INSTRUCTION_DECODER
>         def_bool y
> diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
> index d6a756a..91e38b3 100644
> --- a/arch/x86/include/asm/syscall.h
> +++ b/arch/x86/include/asm/syscall.h
> @@ -23,6 +23,27 @@
>  typedef void (*sys_call_ptr_t)(void);
>  extern const sys_call_ptr_t sys_call_table[];
>
> +/**
> + * syscall_in_syscall() - are we in a syscall context?
> + * @task: The task to query.
> + * @regs: The task's pt_regs.
> + *
> + * This checks whether we are in a syscall.  If it returns true, then
> + * syscall_get_nr(), etc are usable and the current task is guaranteed
> + * to either die or to go through the syscall exit path when the syscall
> + * is done.
> + *
> + * If it returns false, no particular guarantees are made.  In
> + * particular, a malicious task can issue a syscall that causes
> + * syscall_in_syscall to return false.  Such a syscall won't do much,
> + * but it can still cause tracing code and such to run.
> + */
> +static inline bool syscall_in_syscall(struct task_struct *task,
> +                                     struct pt_regs *regs)
> +{
> +       return regs->orig_ax != -1;

This is insufficient: anything that interrupts an errorentry too early
will incorrectly return true.  Also, the actual IRQ entries seem to
shove the IRQ number into orig_ax.

I'll send a new version.

--Andy

[PATCH 2/2] audit: Syscall auditing lite

[Andy Lutomirski]

AFAICS the main use of syscall auditing is to get syscall
information for syscalls that are already causing another audit
message.

We don't need any of the fancy syscall auditing machinery for that,
though: we can just log this information directly.  This should have
essentially no overhead and it could end up being much easier to use
than auditsc.

This produces messages like this:

audit: type=1123 audit(1401485315.370:2): pid=125 uid=0 auid=4294967295 ses=4294967295 subj=kernel msg='blah blah blah' arch=c000003e syscall=44 a0=3 a1=7fff383feb60 a2=5c a3=0 a4=7fff383feb50 a5=c

The new fields (arch, syscall, and a0..a5) will only be logged if we
are in a syscall but we aren't otherwise building an auditsc context.

This is only supported on x86 for now.  Other architectures can get
this if they implement syscall_in_syscall.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 kernel/audit.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 47845c5..8509d00 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -67,6 +67,10 @@
 #include <linux/pid_namespace.h>
 #include <net/netns/generic.h>
 
+#ifdef CONFIG_HAVE_SYSCALL_IN_SYSCALL
+#include <asm/syscall.h>
+#endif
+
 #include "audit.h"
 
 /* No auditing will take place until audit_initialized == AUDIT_INITIALIZED.
@@ -1897,6 +1901,40 @@ out:
 	kfree(name);
 }
 
+#ifdef CONFIG_HAVE_SYSCALL_IN_SYSCALL
+/**
+ * audit_log_missing_context - append otherwise-missing context
+ * @ab: the audit_buffer
+ *
+ * If syscall auditing is unavailable, try to log syscall context
+ * information anyway.
+ */
+static void audit_log_missing_context(struct audit_buffer *ab)
+{
+	struct task_struct *tsk = current;
+	struct pt_regs *regs = current_pt_regs();
+	unsigned long args[6];
+
+	if (!syscall_in_syscall(tsk, regs))
+		return;
+
+	if (ab->ctx && ab->ctx->in_syscall)
+		return;  /* Let audit_log_exit log the context. */
+
+	syscall_get_arguments(tsk, regs, 0, 6, args);
+
+	audit_log_format(ab, " arch=%x syscall=%d a0=%lx a1=%lx a2=%lx a3=%lx a4=%lx a5=%lx",
+			 (unsigned int)syscall_get_arch(),
+			 syscall_get_nr(tsk, regs),
+			 args[0], args[1], args[2], args[3], args[4], args[5]);
+}
+#else
+static void audit_log_missing_context(struct audit_buffer *ab)
+{
+	/* We need arch support to do this reliably, so don't even try. */
+}
+#endif
+
 /**
  * audit_log_end - end one audit record
  * @ab: the audit_buffer
@@ -1913,7 +1951,11 @@ void audit_log_end(struct audit_buffer *ab)
 	if (!audit_rate_check()) {
 		audit_log_lost("rate limit exceeded");
 	} else {
-		struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
+		struct nlmsghdr *nlh;
+
+		audit_log_missing_context(ab);
+
+		nlh = nlmsg_hdr(ab->skb);
 		nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN;
 
 		if (audit_pid) {
-- 
1.9.3