Messages in this thread | | | From | Paul Moore <> | Date | Tue, 23 May 2023 15:59:25 -0400 | Subject | Re: [PATCH] audit: check syscall bitmap on entry to avoid extra work |
| |
On Tue, May 23, 2023 at 2:16 PM Ivan Babrou <ivan@cloudflare.com> 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@cloudflare.com> > --- > include/linux/audit.h | 21 +++++++++++++++++++++ > kernel/auditfilter.c | 32 ++++++++++++++++++++++++++++---- > kernel/auditsc.c | 27 +++++++++++---------------- > 3 files changed, 60 insertions(+), 20 deletions(-)
-- paul-moore.com
| |