Re: [PATCH] audit: check syscall bitmap on entry to avoid extra work

[Paul Moore]

On Tue, May 23, 2023 at 2:16 PM Ivan Babrou <ivan@xxxxxxxxxxxxxx> wrote:
> Currently audit subsystem arms itself as long as there are rules present,
> which means that on every syscall exit all rules are evaluated, even
> if they don't match the syscall to begin with. For setups where
> there are no rules that can match any syscall, this means that
> the CPU price needs to be paid when it's not necessary.
>
> This patch introduces a bitmap for syscalls that is maintained
> when rules are inserted and removed. For every syscall we maintain
> a bit indicating whether it needs to be audited at all, which is then
> checked at syscall entry. If the are no rules matching a syscall,
> extra cost of checking all the rules is avoided.
>
> Consider the following set of 10 audit rules as a benchmark:
>
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/0 -F key=BENCH0
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/1 -F key=BENCH1
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/2 -F key=BENCH2
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/3 -F key=BENCH3
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/4 -F key=BENCH4
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/5 -F key=BENCH5
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/6 -F key=BENCH6
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/7 -F key=BENCH7
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/8 -F key=BENCH8
>     -a always,exit -F arch=b64 -S unlinkat,linkat,renameat,openat,renameat2 -F perm=wa -F dir=/tmp/audit-bench/9 -F key=BENCH9
>
> We can use the following benchmark to run unrelated syscalls:
>
>     #include <sys/stat.h>
>     #include <unistd.h>
>     #include <stdio.h>
>
>     #define GETPID_COUNT 100 * 1000
>     #define STAT_COUNT 100 * 1000
>
>     pid_t bench_getpid()
>     {
>         pid_t pid;
>
>         for (int i = 0; i < GETPID_COUNT; i++)
>         {
>             pid = getpid();
>         }
>
>         return pid;
>     }
>
>     struct stat bench_stat()
>     {
>         struct stat statbuf;
>
>         for (int i = 0; i < STAT_COUNT; i++)
>         {
>             stat("/etc/passwd", &statbuf);
>         }
>
>         return statbuf;
>     }
>
>     int main()
>     {
>         pid_t pid = bench_getpid();
>         struct stat statbuf = bench_stat();
>
>         printf("pid = %d, size = %d\n", pid, statbuf.st_size);
>     }
>
> Here we run 100k `getpid()` calls and 100k `stat()` calls, which are not
> covered by any of the audit rules installed on the system.
>
> When running without any rules present, but with auditd running, flamegraphs
> show ~5% of CPU time spent in audit_* code. If we install the rules mentioned
> above, this number jumps to ~24%. With this patch applied, the number is once
> again down to 5%, which is what one would expect.

Before seriously considering something like this, I would really like
to see some time put into profiling the original overhead and some
designs on how that could be improved.  Without that, patches like
this look like drive-by band-aids which have already caused enough
headaches for audit maintenance.

> There's extra cost of maintaining the bitmap when rules are changed,
> but it's negligible compared to CPU savings from cheaper syscalls.
>
> Signed-off-by: Ivan Babrou <ivan@xxxxxxxxxxxxxx>
> ---
>  include/linux/audit.h | 21 +++++++++++++++++++++
>  kernel/auditfilter.c  | 32 ++++++++++++++++++++++++++++----
>  kernel/auditsc.c      | 27 +++++++++++----------------
>  3 files changed, 60 insertions(+), 20 deletions(-)

-- 
paul-moore.com