* [RFC,v3,1/1] audit: speed up syscall rule filtering
@ 2021-01-24 13:04 yang.yang29
2021-01-28 2:51 ` Paul Moore
0 siblings, 1 reply; 2+ messages in thread
From: yang.yang29 @ 2021-01-24 13:04 UTC (permalink / raw)
To: paul; +Cc: linux-audit, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 5944 bytes --]
From 85b3eccf7f12b091b78cc5ba8abfaf759cf0334e Mon Sep 17 00:00:00 2001
From: Yang Yang <yang.yang29@zte.com.cn>
Date: Sun, 24 Jan 2021 20:40:50 +0800
Subject: [PATCH] audit: speed up syscall rule filtering
audit_filter_syscall() traverses struct list_head audit_filter_list to find
out whether current syscall match one rule. This takes o(n), which is not
necessary, specially for user who add a very few syscall rules. On the other
hand, user may not much care about rule add/delete speed. So do o(n)
calculates when rule changes, and ease the burden of audit_filter_syscall().
Define audit_rule_syscall_mask[NR_syscalls], every element stands for
one syscall.audit_rule_syscall_mask[n] == 0 indicates no rule cares about
syscall n, so we can avoid unnecessary calling audit_filter_syscall().
audit_rule_syscall_mask[n] > 0 indicates at least one rule cares about
syscall n, then calls audit_filter_syscall(). Update
audit_rule_syscall_mask[n] when syscall rule changes.
Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
---
include/linux/audit.h | 3 +++
kernel/auditfilter.c | 4 ++++
kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++----
3 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 82b7c11..867284c 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -302,6 +302,7 @@ extern void audit_seccomp(unsigned long syscall, long signr, int code);
extern void audit_seccomp_actions_logged(const char *names,
const char *old_names, int res);
extern void __audit_ptrace(struct task_struct *t);
+extern void audit_rule_syscall_mask_update(struct audit_krule rule, bool add);
static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
{
@@ -599,6 +600,8 @@ static inline void audit_seccomp(unsigned long syscall, long signr, int code)
static inline void audit_seccomp_actions_logged(const char *names,
const char *old_names, int res)
{ }
+static inline void audit_rule_syscall_mask_update(struct audit_krule rule, bool add)
+{}
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{ }
static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 333b3bc..0b2a8d5 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -957,6 +957,8 @@ static inline int audit_add_rule(struct audit_entry *entry)
return err;
}
+ audit_rule_syscall_mask_update(entry->rule, true);
+
if (watch) {
/* audit_filter_mutex is dropped and re-taken during this call */
err = audit_add_watch(&entry->rule, &list);
@@ -1035,6 +1037,8 @@ int audit_del_rule(struct audit_entry *entry)
goto out;
}
+ audit_rule_syscall_mask_update(e->rule, false);
+
if (e->rule.watch)
audit_remove_watch_rule(&e->rule);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index ce8c9e2..1b8ff4e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -97,6 +97,9 @@ int audit_n_rules;
/* determines whether we collect data for signals sent */
int audit_signals;
+/* whether one syscall needs to be audited */
+u32 audit_rule_syscall_mask[NR_syscalls] = {0};
+
struct audit_aux_data {
struct audit_aux_data *next;
int type;
@@ -799,6 +802,29 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
return rule->mask[word] & bit;
}
+/**
+ * audit_rule_syscall_mask_update - update syscall mask when audit rule changes
+ * @rule: audit rule
+ * @add: add rule or delete
+ *
+ * Caller must hold audit_filter_mutex to prevent stale data.
+ */
+void audit_rule_syscall_mask_update(struct audit_krule rule, bool add)
+{
+ int i;
+
+ if (rule.listnr == AUDIT_FILTER_EXIT && !rule.watch && !rule.tree) {
+ for (i = 0; i < NR_syscalls; i++) {
+ if (unlikely(audit_in_mask(&rule, i))) {
+ if (add == true)
+ audit_rule_syscall_mask[i]++;
+ else
+ audit_rule_syscall_mask[i]--;
+ }
+ }
+ }
+}
+
/* At syscall entry and exit time, this filter is called if the
* audit_state is not low enough that auditing cannot take place, but is
* also not high enough that we already know we have to write an audit
@@ -1627,8 +1653,9 @@ void __audit_free(struct task_struct *tsk)
context->return_valid = AUDITSC_INVALID;
context->return_code = 0;
- audit_filter_syscall(tsk, context,
- &audit_filter_list[AUDIT_FILTER_EXIT]);
+ if (unlikely(audit_rule_syscall_mask[context->major]))
+ audit_filter_syscall(tsk, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
audit_filter_inodes(tsk, context);
if (context->current_state == AUDIT_RECORD_CONTEXT)
audit_log_exit();
@@ -1735,8 +1762,9 @@ void __audit_syscall_exit(int success, long return_code)
else
context->return_code = return_code;
- audit_filter_syscall(current, context,
- &audit_filter_list[AUDIT_FILTER_EXIT]);
+ if (unlikely(audit_rule_syscall_mask[context->major]))
+ audit_filter_syscall(current, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
audit_filter_inodes(current, context);
if (context->current_state == AUDIT_RECORD_CONTEXT)
audit_log_exit();
--
2.15.2
[-- Attachment #2: Type: text/plain, Size: 102 bytes --]
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC,v3,1/1] audit: speed up syscall rule filtering
2021-01-24 13:04 [RFC,v3,1/1] audit: speed up syscall rule filtering yang.yang29
@ 2021-01-28 2:51 ` Paul Moore
0 siblings, 0 replies; 2+ messages in thread
From: Paul Moore @ 2021-01-28 2:51 UTC (permalink / raw)
To: yang.yang29; +Cc: linux-audit, linux-kernel
On Sun, Jan 24, 2021 at 8:04 AM <yang.yang29@zte.com.cn> wrote:
>
> From 85b3eccf7f12b091b78cc5ba8abfaf759cf0334e Mon Sep 17 00:00:00 2001
> From: Yang Yang <yang.yang29@zte.com.cn>
> Date: Sun, 24 Jan 2021 20:40:50 +0800
> Subject: [PATCH] audit: speed up syscall rule filtering
> audit_filter_syscall() traverses struct list_head audit_filter_list to find
> out whether current syscall match one rule. This takes o(n), which is not
> necessary, specially for user who add a very few syscall rules. On the other
> hand, user may not much care about rule add/delete speed. So do o(n)
> calculates when rule changes, and ease the burden of audit_filter_syscall().
>
> Define audit_rule_syscall_mask[NR_syscalls], every element stands for
> one syscall.audit_rule_syscall_mask[n] == 0 indicates no rule cares about
> syscall n, so we can avoid unnecessary calling audit_filter_syscall().
> audit_rule_syscall_mask[n] > 0 indicates at least one rule cares about
> syscall n, then calls audit_filter_syscall(). Update
> audit_rule_syscall_mask[n] when syscall rule changes.
>
> Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
> ---
> include/linux/audit.h | 3 +++
> kernel/auditfilter.c | 4 ++++
> kernel/auditsc.c | 36 ++++++++++++++++++++++++++++++++----
> 3 files changed, 39 insertions(+), 4 deletions(-)
...
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index ce8c9e2..1b8ff4e 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1627,8 +1653,9 @@ void __audit_free(struct task_struct *tsk)
> context->return_valid = AUDITSC_INVALID;
> context->return_code = 0;
>
> - audit_filter_syscall(tsk, context,
> - &audit_filter_list[AUDIT_FILTER_EXIT]);
> + if (unlikely(audit_rule_syscall_mask[context->major]))
> + audit_filter_syscall(tsk, context,
> + &audit_filter_list[AUDIT_FILTER_EXIT]);
> audit_filter_inodes(tsk, context);
> if (context->current_state == AUDIT_RECORD_CONTEXT)
> audit_log_exit();
> @@ -1735,8 +1762,9 @@ void __audit_syscall_exit(int success, long return_code)
> else
> context->return_code = return_code;
>
> - audit_filter_syscall(current, context,
> - &audit_filter_list[AUDIT_FILTER_EXIT]);
> + if (unlikely(audit_rule_syscall_mask[context->major]))
> + audit_filter_syscall(current, context,
> + &audit_filter_list[AUDIT_FILTER_EXIT]);
> audit_filter_inodes(current, context);
> if (context->current_state == AUDIT_RECORD_CONTEXT)
> audit_log_exit();
Looking at this revision I believe I may not have been as clear as I
should have been with my last suggestion. Let me try to do better
here.
Thus far I'm not very happy with the audit_rule_syscall_mask[]
additions; it looks both wasteful and inelegant to me at the moment.
I would much rather see if we can improve the existing code by fixing
inefficiencies in how we handle the rule filtering. This is why my
previous comments suggested looking at the audit_filter_syscall() and
audit_filter_inodes() calls in __audit_free() and
__audit_syscall_exit(), the latter of course being more important due
to its frequency.
In both cases an audit_filter_inode() AUDIT_RECORD_CONTEXT decision
takes precedence over any audit_filter_syscall() decision due to the
code being structured as so:
audit_filter_syscall(...);
audit_filter_inodes(...);
if (state == AUDIT_RECORD_CONTEXT)
audit_log_exit();
... my suggestion is to investigate what performance benefits might be
had by leveraging this precedence, for example:
audit_filter_inodes(...);
if (state != AUDIT_RECORD_CONTEXT)
audit_filter_syscall(...);
if (state == AUDIT_RECORD_CONTEXT)
audit_log_exit();
... of course I would expect the performance to be roughly the same
when there is no matching rule, but I think there would be a
performance when in those cases where a watched inode triggers an
audit rule.
Beyond that, there is probably work we could do to combine some
aspects of audit_filter_syscall() and audit_filter_inodes() to
eliminate some redundancy, e.g. reduce the number of audit_in_mask()
calls. Actually looking a bit closer there are a number of
improvements that could likely be made, some might have some
performance impacts.
Let me know if you are going to pursue the suggestion above about
reordering the audit_filter_*() functions as I'll hold off on the
other changes.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-28 2:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-24 13:04 [RFC,v3,1/1] audit: speed up syscall rule filtering yang.yang29
2021-01-28 2:51 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).