On 2016-06-16 16:54, Paul Moore wrote: > On Tue, Jun 14, 2016 at 5:04 PM, Richard Guy Briggs <r...@redhat.com> wrote: > > RFE: add additional fields for use in audit filter exclude rules > > https://github.com/linux-audit/audit-kernel/issues/5 > > > > Re-factor and combine audit_filter_type() with audit_filter_user() to > > use audit_filter_user_rules() to enable the exclude filter to > > additionally filter on PID, UID, GID, AUID, LOGINUID_SET, SUBJ_*. > > > > The process of combining the similar audit_filter_user() and > > audit_filter_type() functions, required inverting the meaning and > > including the ALWAYS action of the latter. > > > > Keep the check to quit early if the list is empty. > > > > Signed-off-by: Richard Guy Briggs <r...@redhat.com> > > --- > > v2: combine audit_filter_user() and audit_filter_type() into > > audit_filter(). > > --- > > > > include/linux/audit.h | 2 -- > > kernel/audit.c | 4 ++-- > > kernel/audit.h | 2 ++ > > kernel/auditfilter.c | 46 ++++++++++------------------------------------ > > 4 files changed, 14 insertions(+), 40 deletions(-) > > Patches that remove more code than the add always make me happy :)
Ok, see v3, instead of -26, how about -41? > Comment below ... ... > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index 32cdafb..539c1d9 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -160,8 +160,6 @@ extern void audit_log_task_info(struct audit_buffer *ab, > > extern int audit_update_lsm_rules(void); > > > > /* Private API (for audit.c only) */ > > -extern int audit_filter_user(int type); > > -extern int audit_filter_type(int type); > > extern int audit_rule_change(int type, __u32 portid, int seq, > > void *data, size_t datasz); > > extern int audit_list_rules_send(struct sk_buff *request_skb, int seq); > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 384374a..2dfaa19 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -914,7 +914,7 @@ static int audit_receive_msg(struct sk_buff *skb, > > struct nlmsghdr *nlh) > > if (!audit_enabled && msg_type != AUDIT_USER_AVC) > > return 0; > > > > - err = audit_filter_user(msg_type); > > + err = audit_filter(msg_type, AUDIT_FILTER_USER); > > if (err == 1) { /* match or error */ > > err = 0; > > if (msg_type == AUDIT_USER_TTY) { > > @@ -1362,7 +1362,7 @@ struct audit_buffer *audit_log_start(struct > > audit_context *ctx, gfp_t gfp_mask, > > if (audit_initialized != AUDIT_INITIALIZED) > > return NULL; > > > > - if (unlikely(audit_filter_type(type))) > > + if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE))) > > return NULL; > > > > if (gfp_mask & __GFP_DIRECT_RECLAIM) { > > diff --git a/kernel/audit.h b/kernel/audit.h > > index cbbe6bb..1879f02 100644 > > --- a/kernel/audit.h > > +++ b/kernel/audit.h > > @@ -327,6 +327,8 @@ extern pid_t audit_sig_pid; > > extern kuid_t audit_sig_uid; > > extern u32 audit_sig_sid; > > > > +extern int audit_filter(int msgtype, unsigned int listtype); > > + > > #ifdef CONFIG_AUDITSYSCALL > > extern int __audit_signal_info(int sig, struct task_struct *t); > > static inline int audit_signal_info(int sig, struct task_struct *t) > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > > index 96c9a1b..f90c042 100644 > > --- a/kernel/auditfilter.c > > +++ b/kernel/auditfilter.c > > @@ -1349,50 +1349,24 @@ static int audit_filter_user_rules(struct > > audit_krule *rule, int type, > > return 1; > > } > > > > -int audit_filter_user(int type) > > +int audit_filter(int msgtype, unsigned int listtype) > > { > > enum audit_state state = AUDIT_DISABLED; > > struct audit_entry *e; > > - int rc, ret; > > - > > - ret = 1; /* Audit by default */ > > + int rc, result = 1; /* Audit by default */ > > > > rcu_read_lock(); > > - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], > > list) { > > - rc = audit_filter_user_rules(&e->rule, type, &state); > > - if (rc) { > > - if (rc > 0 && state == AUDIT_DISABLED) > > - ret = 0; > > - break; > > - } > > - } > > - rcu_read_unlock(); > > - > > - return ret; > > -} > > - > > -int audit_filter_type(int type) > > -{ > > - struct audit_entry *e; > > - int result = 0; > > - > > - rcu_read_lock(); > > - if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE])) > > + if (list_empty(&audit_filter_list[listtype])) > > goto unlock_and_return; > > > > - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE], > > - list) { > > - int i; > > - for (i = 0; i < e->rule.field_count; i++) { > > - struct audit_field *f = &e->rule.fields[i]; > > - if (f->type == AUDIT_MSGTYPE) { > > - result = audit_comparator(type, f->op, > > f->val); > > - if (!result) > > - break; > > - } > > + list_for_each_entry_rcu(e, &audit_filter_list[listtype], list) { > > + rc = audit_filter_user_rules(&e->rule, msgtype, &state); > > Any reason not to do away with audit_filter_user_rules() and just > insert the code here? As far as I can tell there are no other callers > ... Effort? It was a bit messy pulling it in, but with a bit of effort to clear out some accumulated cruft the result appears clearer to me. Was it worth it? > > + if (rc) { > > + if (rc > 0 && ((state == AUDIT_DISABLED) || > > + (listtype == AUDIT_FILTER_TYPE))) > > + result = 0; > > + break; > > } > > - if (result) > > - goto unlock_and_return; > > } > > unlock_and_return: > > rcu_read_unlock(); > > paul moore - RGB -- Richard Guy Briggs <r...@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635