lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <a8356e46aed7213128e84a888170391adb6afe30.1391449825.git.luto@amacapital.net>
Date:	Mon,  3 Feb 2014 09:53:23 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	linux-audit@...hat.com, linux-kernel@...r.kernel.org
Cc:	Andy Lutomirski <luto@...capital.net>,
	Andi Kleen <andi@...stfloor.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Steve Grubb <sgrubb@...hat.com>, Eric Paris <eparis@...hat.com>
Subject: [PATCH] audit: Only use the syscall slowpath when syscall audit rules exist

This toggles TIF_SYSCALL_AUDIT as needed when rules change instead of
leaving it set whenever rules might be set in the future.  This reduces
syscall latency from >60ns to closer to 40ns on my laptop.

Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Steve Grubb <sgrubb@...hat.com>
Cc: Eric Paris <eparis@...hat.com>
Signed-off-by: Andy Lutomirski <luto@...capital.net>
---

This is not the best-tested patch in the world.  Someone who actually
knows how to use syscall auditing should probably give it a spin.  It
fixes an IMO huge performance issue, though.

 include/linux/audit.h |  9 +++++--
 kernel/auditfilter.c  |  4 ++--
 kernel/auditsc.c      | 65 ++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index a406419..534ba85 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -298,7 +298,9 @@ static inline void audit_mmap_fd(int fd, int flags)
 		__audit_mmap_fd(fd, flags);
 }
 
-extern int audit_n_rules;
+extern void audit_inc_n_rules(void);
+extern void audit_dec_n_rules(void);
+
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */
 static inline int audit_alloc(struct task_struct *task)
@@ -404,7 +406,10 @@ static inline void audit_mmap_fd(int fd, int flags)
 { }
 static inline void audit_ptrace(struct task_struct *t)
 { }
-#define audit_n_rules 0
+static inline void audit_inc_n_rules(void)
+{ }
+static inline void audit_dec_n_rules(void)
+{ }
 #define audit_signals 0
 #endif /* CONFIG_AUDITSYSCALL */
 
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 51f3fd4..0ce531d 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -903,7 +903,7 @@ static inline int audit_add_rule(struct audit_entry *entry)
 	}
 #ifdef CONFIG_AUDITSYSCALL
 	if (!dont_count)
-		audit_n_rules++;
+		audit_inc_n_rules();
 
 	if (!audit_match_signal(entry))
 		audit_signals++;
@@ -955,7 +955,7 @@ static inline int audit_del_rule(struct audit_entry *entry)
 
 #ifdef CONFIG_AUDITSYSCALL
 	if (!dont_count)
-		audit_n_rules--;
+		audit_dec_n_rules();
 
 	if (!audit_match_signal(entry))
 		audit_signals--;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 90594c9..363d0bb 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -79,8 +79,15 @@
 /* no execve audit message should be longer than this (userspace limits) */
 #define MAX_EXECVE_AUDIT_LEN 7500
 
-/* number of audit rules */
-int audit_n_rules;
+/*
+ * number of audit rules
+ *
+ * n_rules_lock protects audit_n_rules.  It also prevents audit_alloc from
+ * racing against changes to audit_n_rules: to change someone else's
+ * audit_context or TIF_SYSCALL_AUDIT, you need write_lock(&n_rules_lock).
+ */
+static DEFINE_RWLOCK(n_rules_lock);
+static int audit_n_rules;
 
 /* determines whether we collect data for signals sent */
 int audit_signals;
@@ -911,6 +918,47 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
 	return context;
 }
 
+void audit_inc_n_rules()
+{
+	struct task_struct *p, *g;
+
+	write_lock(&n_rules_lock);
+
+	if (audit_n_rules++ != 0)
+		goto out;  /* The overall state isn't changing. */
+
+	read_lock(&tasklist_lock);
+	do_each_thread(g, p) {
+		if (p->audit_context)
+			set_tsk_thread_flag(p, TIF_SYSCALL_AUDIT);
+	} while_each_thread(g, p);
+	read_unlock(&tasklist_lock);
+
+out:
+	write_unlock(&n_rules_lock);
+}
+
+void audit_dec_n_rules()
+{
+	struct task_struct *p, *g;
+
+	write_lock(&n_rules_lock);
+	--audit_n_rules;
+	BUG_ON(audit_n_rules < 0);
+
+	if (audit_n_rules != 0)
+		goto out;  /* The overall state isn't changing. */
+
+	read_lock(&tasklist_lock);
+	do_each_thread(g, p) {
+		clear_tsk_thread_flag(p, TIF_SYSCALL_AUDIT);
+	} while_each_thread(g, p);
+	read_unlock(&tasklist_lock);
+
+out:
+	write_unlock(&n_rules_lock);
+}
+
 /**
  * audit_alloc - allocate an audit context block for a task
  * @tsk: task
@@ -931,6 +979,11 @@ int audit_alloc(struct task_struct *tsk)
 
 	state = audit_filter_task(tsk, &key);
 	if (state == AUDIT_DISABLED) {
+		/*
+		 * There is no relevant race against inc/dec_n_rules
+		 * here -- inc won't set TIF_SYSCALL_AUDIT, and, if dec
+		 * clears it redundantly, there's no harm done.
+		 */
 		clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
 		return 0;
 	}
@@ -942,8 +995,14 @@ int audit_alloc(struct task_struct *tsk)
 	}
 	context->filterkey = key;
 
+	read_lock(&n_rules_lock);
 	tsk->audit_context  = context;
-	set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+	if (audit_n_rules)
+		set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+	else
+		clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+	read_unlock(&n_rules_lock);
+
 	return 0;
 }
 
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ