Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932622AbdIGWgi (ORCPT ); Thu, 7 Sep 2017 18:36:38 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:34231 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932587AbdIGWgf (ORCPT ); Thu, 7 Sep 2017 18:36:35 -0400 X-Google-Smtp-Source: ADKCNb51lTYlIOHBt0jpF4gO4CrV5lmfEEyhTRPJm9TQ15K+Bgvv1WUEEXCzL++n2wS9wgadoyXq7kG/ikr8L/PaK7M= MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: References: From: Paul Moore Date: Thu, 7 Sep 2017 18:36:32 -0400 Message-ID: Subject: Re: [PATCH ALT4 V3 2/2] audit: filter PATH records keyed on filesystem magic To: Richard Guy Briggs , sgrubb@redhat.com Cc: linux-kernel@vger.kernel.org, linux-audit@redhat.com, Steven Rostedt Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8990 Lines: 225 On Wed, Aug 23, 2017 at 7:03 AM, Richard Guy Briggs wrote: > Tracefs or debugfs were causing hundreds to thousands of PATH records to > be associated with the init_module and finit_module SYSCALL records on a > few modules when the following rule was in place for startup: > -a always,exit -F arch=x86_64 -S init_module -F key=mod-load > > Provide a method to ignore these large number of PATH records from > overwhelming the logs if they are not of interest. Introduce a new > filter list "AUDIT_FILTER_FS", with a new field type AUDIT_FSTYPE, > which keys off the filesystem 4-octet hexadecimal magic identifier to > filter specific filesystem PATH records. > > An example rule would look like: > -a never,filesystem -F fstype=0x74726163 -F key=ignore_tracefs > -a never,filesystem -F fstype=0x64626720 -F key=ignore_debugfs > > Arguably the better way to address this issue is to disable tracefs and > debugfs on boot from production systems. > > See: https://github.com/linux-audit/audit-kernel/issues/16 > See: https://github.com/linux-audit/audit-userspace/issues/8 > Test case: https://github.com/linux-audit/audit-testsuite/issues/42 > > Signed-off-by: Richard Guy Briggs > > --- > v3: rebase > v2: convert AUDIT_FILTER_PATH to AUDIT_FILTER_FS > --- > include/uapi/linux/audit.h | 8 ++++++-- > kernel/auditfilter.c | 39 ++++++++++++++++++++++++++++++++------- > kernel/auditsc.c | 23 +++++++++++++++++++++++ > 3 files changed, 61 insertions(+), 9 deletions(-) Both this patch and 1/2 look okay, but I'm not going to merge either until after the merge window closes. Steve, I assume you are still happy with the kernel/userspace interface for this? > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > index 0714a66..be71134 100644 > --- a/include/uapi/linux/audit.h > +++ b/include/uapi/linux/audit.h > @@ -155,8 +155,9 @@ > #define AUDIT_FILTER_WATCH 0x03 /* Apply rule to file system watches */ > #define AUDIT_FILTER_EXIT 0x04 /* Apply rule at syscall exit */ > #define AUDIT_FILTER_TYPE 0x05 /* Apply rule at audit_log_start */ > +#define AUDIT_FILTER_FS 0x06 /* Apply rule at __audit_inode_child */ > > -#define AUDIT_NR_FILTERS 6 > +#define AUDIT_NR_FILTERS 7 > > #define AUDIT_FILTER_PREPEND 0x10 /* Prepend to front of list */ > > @@ -256,6 +257,7 @@ > #define AUDIT_OBJ_LEV_HIGH 23 > #define AUDIT_LOGINUID_SET 24 > #define AUDIT_SESSIONID 25 /* Session ID */ > +#define AUDIT_FSTYPE 26 /* FileSystem Type */ > > /* These are ONLY useful when checking > * at syscall exit time (AUDIT_AT_EXIT). */ > @@ -335,13 +337,15 @@ enum { > #define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND 0x00000008 > #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER 0x00000010 > #define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000020 > +#define AUDIT_FEATURE_BITMAP_FILTER_FS 0x00000040 > > #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \ > AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \ > AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \ > AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \ > AUDIT_FEATURE_BITMAP_SESSIONID_FILTER | \ > - AUDIT_FEATURE_BITMAP_LOST_RESET) > + AUDIT_FEATURE_BITMAP_LOST_RESET | \ > + AUDIT_FEATURE_BITMAP_FILTER_FS) > > /* deprecated: AUDIT_VERSION_* */ > #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 0b0aa58..4a1758a 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -56,7 +56,8 @@ struct list_head audit_filter_list[AUDIT_NR_FILTERS] = { > LIST_HEAD_INIT(audit_filter_list[3]), > LIST_HEAD_INIT(audit_filter_list[4]), > LIST_HEAD_INIT(audit_filter_list[5]), > -#if AUDIT_NR_FILTERS != 6 > + LIST_HEAD_INIT(audit_filter_list[6]), > +#if AUDIT_NR_FILTERS != 7 > #error Fix audit_filter_list initialiser > #endif > }; > @@ -67,6 +68,7 @@ struct list_head audit_filter_list[AUDIT_NR_FILTERS] = { > LIST_HEAD_INIT(audit_rules_list[3]), > LIST_HEAD_INIT(audit_rules_list[4]), > LIST_HEAD_INIT(audit_rules_list[5]), > + LIST_HEAD_INIT(audit_rules_list[6]), > }; > > DEFINE_MUTEX(audit_filter_mutex); > @@ -263,6 +265,7 @@ static int audit_match_signal(struct audit_entry *entry) > #endif > case AUDIT_FILTER_USER: > case AUDIT_FILTER_TYPE: > + case AUDIT_FILTER_FS: > ; > } > if (unlikely(rule->action == AUDIT_POSSIBLE)) { > @@ -338,6 +341,21 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) > entry->rule.listnr != AUDIT_FILTER_USER) > return -EINVAL; > break; > + case AUDIT_FSTYPE: > + if (entry->rule.listnr != AUDIT_FILTER_FS) > + return -EINVAL; > + break; > + } > + > + switch(entry->rule.listnr) { > + case AUDIT_FILTER_FS: > + switch(f->type) { > + case AUDIT_FSTYPE: > + case AUDIT_FILTERKEY: > + break; > + default: > + return -EINVAL; > + } > } > > switch(f->type) { > @@ -391,6 +409,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) > return -EINVAL; > /* FALL THROUGH */ > case AUDIT_ARCH: > + case AUDIT_FSTYPE: > if (f->op != Audit_not_equal && f->op != Audit_equal) > return -EINVAL; > break; > @@ -910,10 +929,13 @@ static inline int audit_add_rule(struct audit_entry *entry) > #ifdef CONFIG_AUDITSYSCALL > int dont_count = 0; > > - /* If either of these, don't count towards total */ > - if (entry->rule.listnr == AUDIT_FILTER_USER || > - entry->rule.listnr == AUDIT_FILTER_TYPE) > + /* If any of these, don't count towards total */ > + switch(entry->rule.listnr) { > + case AUDIT_FILTER_USER: > + case AUDIT_FILTER_TYPE: > + case AUDIT_FILTER_FS: > dont_count = 1; > + } > #endif > > mutex_lock(&audit_filter_mutex); > @@ -989,10 +1011,13 @@ int audit_del_rule(struct audit_entry *entry) > #ifdef CONFIG_AUDITSYSCALL > int dont_count = 0; > > - /* If either of these, don't count towards total */ > - if (entry->rule.listnr == AUDIT_FILTER_USER || > - entry->rule.listnr == AUDIT_FILTER_TYPE) > + /* If any of these, don't count towards total */ > + switch(entry->rule.listnr) { > + case AUDIT_FILTER_USER: > + case AUDIT_FILTER_TYPE: > + case AUDIT_FILTER_FS: > dont_count = 1; > + } > #endif > > mutex_lock(&audit_filter_mutex); > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 11848df..ce6cbda 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1871,10 +1871,33 @@ void __audit_inode_child(struct inode *parent, > struct inode *inode = d_backing_inode(dentry); > const char *dname = dentry->d_name.name; > struct audit_names *n, *found_parent = NULL, *found_child = NULL; > + struct audit_entry *e; > + struct list_head *list = &audit_filter_list[AUDIT_FILTER_FS]; > + int i; > > if (!context->in_syscall) > return; > > + rcu_read_lock(); > + if (!list_empty(list)) { > + list_for_each_entry_rcu(e, list, list) { > + for (i = 0; i < e->rule.field_count; i++) { > + struct audit_field *f = &e->rule.fields[i]; > + > + if (f->type == AUDIT_FSTYPE) { > + if (audit_comparator(parent->i_sb->s_magic, > + f->op, f->val)) { > + if (e->rule.action == AUDIT_NEVER) { > + rcu_read_unlock(); > + return; > + } > + } > + } > + } > + } > + } > + rcu_read_unlock(); > + > if (inode) > handle_one(inode); > > -- > 1.7.1 > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit -- paul moore www.paul-moore.com