From: Paul Moore <paul@paul-moore.com> To: yang.yang29@zte.com.cn Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [RFC,v2,1/1] audit: speed up syscall rule match while exiting syscall Date: Fri, 22 Jan 2021 12:02:35 -0500 [thread overview] Message-ID: <CAHC9VhQuBFv83G05QQJLuOV903sFX7ijqjgpCvWH-dk7cbJCjg@mail.gmail.com> (raw) In-Reply-To: <202101212154272626110@zte.com.cn> On Thu, Jan 21, 2021 at 8:54 AM <yang.yang29@zte.com.cn> wrote: > > From 72f3ecde58edb03d76cb359607fef98c1663d481 Mon Sep 17 00:00:00 2001 > From: Yang Yang <yang.yang29@zte.com.cn> > Date: Thu, 21 Jan 2021 21:05:04 +0800 > Subject: [PATCH] [RFC,v2,1/1] speed up syscall rule match while exiting syscall > 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) > calculate at rule changing, and ease the burden of audit_filter_syscall(). > > Define audit_syscall[NR_syscalls], every element stands for one syscall. > audit_filter_syscall() checks audit_syscall[NR_syscalls]. > audit_syscall[n] == 0 indicates no rule audit syscall n, do a quick exit. > audit_syscall[n] > 0 indicates at least one rule audit syscall n. > audit_syscall[n] update when syscall rule changes. > > Signed-off-by: Yang Yang <yang.yang29@zte.com.cn> > --- > include/linux/audit.h | 2 ++ > kernel/audit.c | 4 ++++ > kernel/auditfilter.c | 30 ++++++++++++++++++++++++++++++ > kernel/auditsc.c | 5 ++++- > 4 files changed, 40 insertions(+), 1 deletion(-) ... > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 333b3bc..9d3e703 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -926,6 +926,28 @@ static struct audit_entry *audit_find_rule(struct audit_entry *entry, > static u64 prio_low = ~0ULL/2; > static u64 prio_high = ~0ULL/2 - 1; > > +#ifdef CONFIG_AUDITSYSCALL > +static inline void update_auditing_syscall(struct audit_krule rule, bool add) > +{ > + int i; > + > + /* syscall rule with type AUDIT_FILTER_EXIT */ > + if (rule.listnr == AUDIT_FILTER_EXIT && !rule.watch && !rule.tree) { > + for (i = 0; i < NR_syscalls; i++) { > + /* whether this rule include one syscall */ > + if (unlikely(audit_in_mask(&rule, i))) { > + if (add == true) > + auditing_syscall[i]++; > + else > + auditing_syscall[i]--; > + } > + } > + } > + > + return; > +} > +#endif > + > /* Add rule to given filterlist if not a duplicate. */ > static inline int audit_add_rule(struct audit_entry *entry) > { > @@ -957,6 +979,10 @@ static inline int audit_add_rule(struct audit_entry *entry) > return err; > } > > +#ifdef CONFIG_AUDITSYSCALL > + update_auditing_syscall(entry->rule, true); > +#endif I'm going to reply to your other email where we are discussing the performance of this patch, but I wanted to make one comment about the approach you've taken with the update_auditing_syscall() here. First, naming things is hard, but the chosen name is not a good one in my opinion. Something like audit_rule_syscall_mask_update() would probably be a better fit. Second, in order to minimize preprocessor clutter, it is better to use the following pattern: #ifdef CONFIG_FOO int func(int arg) { /* important stuff */ } #else int func(int arg) { return 0; /* appropriate return value */ } #endif There are probably a few other comments on this patch, but I want us to discuss the performance impacts of this first as I'm not convinced this is a solution we want upstream. -- paul moore www.paul-moore.com
WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com> To: yang.yang29@zte.com.cn Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [RFC, v2, 1/1] audit: speed up syscall rule match while exiting syscall Date: Fri, 22 Jan 2021 12:02:35 -0500 [thread overview] Message-ID: <CAHC9VhQuBFv83G05QQJLuOV903sFX7ijqjgpCvWH-dk7cbJCjg@mail.gmail.com> (raw) In-Reply-To: <202101212154272626110@zte.com.cn> On Thu, Jan 21, 2021 at 8:54 AM <yang.yang29@zte.com.cn> wrote: > > From 72f3ecde58edb03d76cb359607fef98c1663d481 Mon Sep 17 00:00:00 2001 > From: Yang Yang <yang.yang29@zte.com.cn> > Date: Thu, 21 Jan 2021 21:05:04 +0800 > Subject: [PATCH] [RFC,v2,1/1] speed up syscall rule match while exiting syscall > 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) > calculate at rule changing, and ease the burden of audit_filter_syscall(). > > Define audit_syscall[NR_syscalls], every element stands for one syscall. > audit_filter_syscall() checks audit_syscall[NR_syscalls]. > audit_syscall[n] == 0 indicates no rule audit syscall n, do a quick exit. > audit_syscall[n] > 0 indicates at least one rule audit syscall n. > audit_syscall[n] update when syscall rule changes. > > Signed-off-by: Yang Yang <yang.yang29@zte.com.cn> > --- > include/linux/audit.h | 2 ++ > kernel/audit.c | 4 ++++ > kernel/auditfilter.c | 30 ++++++++++++++++++++++++++++++ > kernel/auditsc.c | 5 ++++- > 4 files changed, 40 insertions(+), 1 deletion(-) ... > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 333b3bc..9d3e703 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -926,6 +926,28 @@ static struct audit_entry *audit_find_rule(struct audit_entry *entry, > static u64 prio_low = ~0ULL/2; > static u64 prio_high = ~0ULL/2 - 1; > > +#ifdef CONFIG_AUDITSYSCALL > +static inline void update_auditing_syscall(struct audit_krule rule, bool add) > +{ > + int i; > + > + /* syscall rule with type AUDIT_FILTER_EXIT */ > + if (rule.listnr == AUDIT_FILTER_EXIT && !rule.watch && !rule.tree) { > + for (i = 0; i < NR_syscalls; i++) { > + /* whether this rule include one syscall */ > + if (unlikely(audit_in_mask(&rule, i))) { > + if (add == true) > + auditing_syscall[i]++; > + else > + auditing_syscall[i]--; > + } > + } > + } > + > + return; > +} > +#endif > + > /* Add rule to given filterlist if not a duplicate. */ > static inline int audit_add_rule(struct audit_entry *entry) > { > @@ -957,6 +979,10 @@ static inline int audit_add_rule(struct audit_entry *entry) > return err; > } > > +#ifdef CONFIG_AUDITSYSCALL > + update_auditing_syscall(entry->rule, true); > +#endif I'm going to reply to your other email where we are discussing the performance of this patch, but I wanted to make one comment about the approach you've taken with the update_auditing_syscall() here. First, naming things is hard, but the chosen name is not a good one in my opinion. Something like audit_rule_syscall_mask_update() would probably be a better fit. Second, in order to minimize preprocessor clutter, it is better to use the following pattern: #ifdef CONFIG_FOO int func(int arg) { /* important stuff */ } #else int func(int arg) { return 0; /* appropriate return value */ } #endif There are probably a few other comments on this patch, but I want us to discuss the performance impacts of this first as I'm not convinced this is a solution we want upstream. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2021-01-22 17:14 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-21 13:54 [RFC,v2,1/1] audit: speed up syscall rule match while exiting syscall yang.yang29 2021-01-22 17:02 ` Paul Moore [this message] 2021-01-22 17:02 ` [RFC, v2, 1/1] " Paul Moore
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAHC9VhQuBFv83G05QQJLuOV903sFX7ijqjgpCvWH-dk7cbJCjg@mail.gmail.com \ --to=paul@paul-moore.com \ --cc=linux-audit@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=yang.yang29@zte.com.cn \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.