lkml.org 
[lkml]   [2022]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/3] audit: unify audit_filter_{uring(),inode_name(),syscall()}
On Tue, Sep 27, 2022 at 7:00 PM Ankur Arora <ankur.a.arora@oracle.com> wrote:
>
> audit_filter_uring(), audit_filter_inode_name() are substantially similar
> to audit_filter_syscall(). Move the core logic to __audit_filter() which
> can be parametrized for all three.
>
> On a Skylakex system, getpid() latency (all results aggregated
> across 12 boot cycles):
>
> Min Mean Median Max pstdev
> (ns) (ns) (ns) (ns)
>
> - 173.11 182.51 179.65 202.09 (+- 4.34%)
> + 162.11 175.26 173.71 190.56 (+- 4.33%)
>
> Performance counter stats for 'bin/getpid' (3 runs) go from:
> cycles 706.13 ( +- 4.13% )
> instructions 1654.70 ( +- .06% )
> IPC 2.35 ( +- 4.25% )
> branches 430.99 ( +- .06% )
> branch-misses 0.50 ( +- 2.00% )
> L1-dcache-loads 440.02 ( +- .07% )
> L1-dcache-load-misses 5.22 ( +- 82.75% )
>
> to:
> cycles 678.79 ( +- 4.22% )
> instructions 1657.79 ( +- .05% )
> IPC 2.45 ( +- 4.08% )
> branches 432.00 ( +- .05% )
> branch-misses 0.38 ( +- 23.68% )
> L1-dcache-loads 444.96 ( +- .03% )
> L1-dcache-load-misses 5.13 ( +- 73.09% )
>
> (Both aggregated over 12 boot cycles.)
>
> Unclear if the improvement is just run-to-run variation or because of
> a slightly denser loop (the list parameter in the list_for_each_entry_rcu()
> exit check now comes from a register rather than a constant as before.)
>
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> ---
> kernel/auditsc.c | 86 +++++++++++++++++++++++++-----------------------
> 1 file changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index bf26f47b5226..dd89a52988b0 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -805,6 +805,41 @@ static bool audit_in_mask(const struct audit_krule *rule, unsigned long val)
> return rule->mask[word] & bit;
> }
>
> +/**
> + * __audit_filter - common filter
> + *

Please remove the vertical whitespace between the function description
line and the parameter descriptions.

I'd also suggest renaming this function to "__audit_filter_op(...)"
since we have a few audit filtering functions and a generic
"__audit_filter()" function with no qualification in the name seems
just a bit too generic to not be misleading ... at least I think so.

I also might try to be just a bit more descriptive in the text above,
for example:

"__audit_filter_op - filter helper function for operations (syscall/uring/etc.)"

> + * @tsk: associated task
> + * @ctx: audit context
> + * @list: audit filter list
> + * @op: current syscall/uring_op
> + * @name: name to be filtered (used by audit_filter_inode_name)

I would change this to "@name: audit_name to use in filtering (can be
NULL)" and leave it at that.

> + *
> + * return: 1 if we hit a filter, 0 if we don't

The function header block comment description should be in regular
English sentences. Perhaps something like this:

"Run the audit filters specified in @list against @tsk using @ctx,
@op, and @name as necessary; the caller is responsible for ensuring
that the call is made while the RCU read lock is held. The @name
parameter can be NULL, but all others must be specified. Returns
1/true if the filter finds a match, 0/false if none are found."

> + */
> +static int __audit_filter(struct task_struct *tsk,
> + struct audit_context *ctx,
> + struct list_head *list,
> + unsigned long op,
> + struct audit_names *name)
> +{
> + struct audit_entry *e;
> + enum audit_state state;
> +
> + rcu_read_lock();

I would move the RCU locking to the callers since not every caller requires it.

> + list_for_each_entry_rcu(e, list, list) {
> + if (unlikely(audit_in_mask(&e->rule, op))) {

As discussed in patch 2/3, please remove the unlikely() call.

> + if (audit_filter_rules(tsk, &e->rule, ctx, name,
> + &state, false)) {
> + rcu_read_unlock();
> + ctx->current_state = state;
> + return 1;
> + }
> + }
> + }
> + rcu_read_unlock();
> + return 0;
> +}
> +

--
paul-moore.com

\
 
 \ /
  Last update: 2022-09-29 00:56    [W:0.191 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site