ausearch-test 0.5
by Steve Grubb
Hello,
An updated version of the ausearch test suite was released. Some of the
changes are:
* Fixed handling of encoded fields being passed to ausearch
* Added a --continue command line option to instruct the test to not stop on
the first failure
* Documentation updates better explaining what is being tested
http://people.redhat.com/sgrubb/audit/ausearch-test-0.5.tar.gz
-Steve
10 years, 10 months
[PATCH v2.1] audit: Only use the syscall slowpath when syscall audit rules exist
by Andy Lutomirski
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(a)redhat.com>
Cc: Steve Grubb <sgrubb(a)redhat.com>
Cc: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Andy Lutomirski <luto(a)amacapital.net>
---
This brown paper bag release is brought to you by git commit's -a flag.
Changes from v2: Contains the correct patch
Changes from v1:
- For new tasks, set flags in a new audit_sync_flags callback instead of
in audit_alloc (thanks, Oleg).
- Rework locking.
- Use irqsave/irqrestore to avoid having to think about who else might have
taken spinlocks.
include/linux/audit.h | 11 ++++++++--
kernel/auditfilter.c | 4 ++--
kernel/auditsc.c | 57 +++++++++++++++++++++++++++++++++++++++++++++------
kernel/fork.c | 3 +++
4 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index a406419..a81f498 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 void audit_sync_flags(struct task_struct *tsk);
extern int audit_signals;
#else /* CONFIG_AUDITSYSCALL */
static inline int audit_alloc(struct task_struct *task)
@@ -404,7 +406,12 @@ 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)
+{ }
+static inline void audit_sync_flags(struct task_struct *tsk)
+{ }
#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..cd44c88 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -79,8 +79,13 @@
/* 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
+ *
+ * To change this, you must hold audit_filter_mutex *and* have a read lock
+ * on tasklist_lock.
+ */
+static int audit_n_rules;
/* determines whether we collect data for signals sent */
int audit_signals;
@@ -911,6 +916,40 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
return context;
}
+void audit_inc_n_rules()
+{
+ struct task_struct *p, *g;
+ unsigned long flags;
+
+ read_lock_irqsave(&tasklist_lock, flags);
+ if (audit_n_rules++ == 0) {
+ do_each_thread(g, p) {
+ if (p->audit_context)
+ set_tsk_thread_flag(p, TIF_SYSCALL_AUDIT);
+ } while_each_thread(g, p);
+ }
+ read_unlock_irqrestore(&tasklist_lock, flags);
+}
+
+void audit_dec_n_rules()
+{
+ struct task_struct *p, *g;
+ unsigned long flags;
+
+ read_lock_irqsave(&tasklist_lock, flags);
+
+ --audit_n_rules;
+ BUG_ON(audit_n_rules < 0);
+
+ if (audit_n_rules == 0) {
+ do_each_thread(g, p) {
+ clear_tsk_thread_flag(p, TIF_SYSCALL_AUDIT);
+ } while_each_thread(g, p);
+ }
+
+ read_unlock_irqrestore(&tasklist_lock, flags);
+}
+
/**
* audit_alloc - allocate an audit context block for a task
* @tsk: task
@@ -930,10 +969,8 @@ int audit_alloc(struct task_struct *tsk)
return 0; /* Return if not auditing. */
state = audit_filter_task(tsk, &key);
- if (state == AUDIT_DISABLED) {
- clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+ if (state == AUDIT_DISABLED)
return 0;
- }
if (!(context = audit_alloc_context(state))) {
kfree(key);
@@ -943,10 +980,18 @@ int audit_alloc(struct task_struct *tsk)
context->filterkey = key;
tsk->audit_context = context;
- set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
return 0;
}
+void audit_sync_flags(struct task_struct *tsk)
+{
+ /* The caller has a write lock on tasklist_lock. */
+ if (audit_n_rules && tsk->audit_context)
+ set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+ else
+ clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
+}
+
static inline void audit_free_context(struct audit_context *context)
{
audit_free_names(context);
diff --git a/kernel/fork.c b/kernel/fork.c
index dfa736c..3f28173 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1479,6 +1479,9 @@ static struct task_struct *copy_process(unsigned long clone_flags,
total_forks++;
spin_unlock(¤t->sighand->siglock);
+
+ audit_sync_flags(p);
+
write_unlock_irq(&tasklist_lock);
proc_fork_connector(p);
cgroup_post_fork(p);
--
1.8.5.3
10 years, 10 months
[PATCH] loginuid change logging details
by Richard Guy Briggs
I missed posting this before the holidays. I discovered this while adding
other information to other message types.
It seemed to me that loginuid changes were significantly missing context
references. This patch adds that. Is this sufficient, or is there more
information missing too? If this is sufficient, stop reading this cover letter
and review the patch. If it is not sufficient, keep reading below...
The question has been raised that perhaps we should be switching this to use
audit_log_task_info() istead which adds a whole lot more information about this
task.
In the existing message
pid
uid
are already given, before
old-auid
new-auid
old-ses
new-ses
.
The function audit_log_task_info() gives:
ppid
pid
auid
uid
gid
euid
suid
fsuid
egid
sgid
fsgid
tty
ses
comm
exe
res
.
So,
pid
uid
are in the right order, along with
new-auid (auid)
new-ses (ses)
but if we give the
old-auid
old-ses
values first, then call audit_log_task_info(), the old values will preceed
pid
uid
.
Is this re-ordering acceptable to gain more information and reduce code duplicity?
Richard Guy Briggs (1):
audit: log task context when setting loginuid
kernel/auditsc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
10 years, 10 months
[PATCH] audit: Only use the syscall slowpath when syscall audit rules exist
by Andy Lutomirski
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(a)redhat.com>
Cc: Steve Grubb <sgrubb(a)redhat.com>
Cc: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Andy Lutomirski <luto(a)amacapital.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
10 years, 10 months
[PATCH v2] audit: Only use the syscall slowpath when syscall audit rules exist
by Andy Lutomirski
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(a)redhat.com>
Cc: Steve Grubb <sgrubb(a)redhat.com>
Cc: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Andy Lutomirski <luto(a)amacapital.net>
---
Changes from v1:
- For new tasks, set flags in a new audit_sync_flags callback instead of
in audit_alloc (thanks, Oleg).
- Rework locking.
- Use irqsave/irqrestore to avoid having to think about who else might have
taken spinlocks.
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
10 years, 10 months
Why is syscall auditing on with no rules?
by Andy Lutomirski
On a stock Fedora installation:
$ sudo auditctl -l
No rules
Nonetheless TIF_SYSCALL_AUDIT is set and the __audit_syscall_entry and
__audit_syscall_exit account for >20% of syscall overhead according to
perf.
This sucks. Unless I'm missing something, syscall auditing is *off*.
How hard would it be to arrange for TIF_SYSCALL_AUDIT to be cleared
when there are no syscall rules?
(This is extra bad in kernels before 3.13, where the clear call for
TIF_SYSCALL_AUDIT was completely missing.)
--Andy
10 years, 10 months
