Messages in this thread | | | From | Paul Moore <> | Date | Thu, 16 Jun 2016 16:54:42 -0400 | Subject | Re: [PATCH v2] audit: add fields to exclude filter by reusing user filter |
| |
On Tue, Jun 14, 2016 at 5:04 PM, Richard Guy Briggs <rgb@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 <rgb@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 :)
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 ...
> + 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 www.paul-moore.com
| |