Re: [RFC PATCH 2/2] [WIP] audit: allow other filter list types for AUDIT_DIR

On 2018-05-30 10:45, Ondrej Mosnacek wrote: > This patch allows the AUDIR_DIR field to be used also with the exclude > filter. > > Not-yet-signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com&gt; > --- > kernel/audit.c | 5 +++-- > kernel/audit.h | 32 +++++++++++++++++++++++++++++++- > kernel/audit_tree.c | 4 +++- > kernel/auditfilter.c | 6 +++++- > kernel/auditsc.c | 28 ---------------------------- > 5 files changed, 42 insertions(+), 33 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index e7478cb58079..aac5b6ecc11d 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1333,7 +1333,8 @@ 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(msg_type, AUDIT_FILTER_USER); > + // FIXME: do we need to pass the context here? > + err = audit_filter(msg_type, AUDIT_FILTER_USER, NULL); > if (err == 1) { /* match or error */ > err = 0; > if (msg_type == AUDIT_USER_TTY) { > @@ -1754,7 +1755,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, AUDIT_FILTER_TYPE))) > + if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE, ctx))) > return NULL; > > /* NOTE: don't ever fail/sleep on these two conditions: > diff --git a/kernel/audit.h b/kernel/audit.h > index 214e14948370..43cfeba25802 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -324,13 +324,43 @@ extern void audit_kill_trees(struct list_head *list); > #define audit_kill_trees(list) BUG() > #endif > > +struct audit_tree_refs { > + struct audit_tree_refs *next; > + struct audit_chunk *c[31]; > +}; > + > +/* A utility function to match tree refs: */ > +static inline int match_tree_refs(struct audit_context *ctx, struct audit_tree *tree) > +{ > +#ifdef CONFIG_AUDIT_TREE > + struct audit_tree_refs *p; > + int n; > + if (!tree) > + return 0; > + /* full ones */ > + for (p = ctx->first_trees; p != ctx->trees; p = p->next) { > + for (n = 0; n < 31; n++) > + if (audit_tree_match(p->c[n], tree)) > + return 1; > + } > + /* partial */ > + if (p) { > + for (n = ctx->tree_count; n < 31; n++) > + if (audit_tree_match(p->c[n], tree)) > + return 1; > + } > +#endif > + return 0; > +} > + > extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len); > > 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); > +extern int audit_filter(int msgtype, unsigned int listtype, > + struct audit_context *ctx); > > #ifdef CONFIG_AUDITSYSCALL > extern int audit_signal_info(int sig, struct task_struct *t); > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > index 67e6956c0b61..d4d36389c3d7 100644 > --- a/kernel/audit_tree.c > +++ b/kernel/audit_tree.c > @@ -675,9 +675,11 @@ void audit_trim_trees(void) > > int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op) > { > + if (krule->listnr != AUDIT_FILTER_EXIT && > + krule->listnr != AUDIT_FILTER_TYPE) > + return -EINVAL; > > if (pathname[0] != '/' || > - rule->listnr != AUDIT_FILTER_EXIT || > op != Audit_equal || > rule->inode_f || rule->watch || rule->tree) > return -EINVAL; > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 6db9847ca031..e1d70cb77ea3 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -1309,7 +1309,7 @@ int audit_compare_dname_path(const char *dname, const char *path, int parentlen) > return strncmp(p, dname, dlen); > } > > -int audit_filter(int msgtype, unsigned int listtype) > +int audit_filter(int msgtype, unsigned int listtype, struct audit_context *ctx) > { > struct audit_entry *e; > int ret = 1; /* Audit by default */ > @@ -1363,6 +1363,10 @@ int audit_filter(int msgtype, unsigned int listtype) > if (f->op == Audit_not_equal) > result = !result; > break; > + case AUDIT_DIR: > + if (ctx) > + result = match_tree_refs(ctx, e->rule.tree); I don't see why you need to send a context to audit_filter, since the rest of the function assumes the current task you can just use audit_context() to hand to match_tree_refs(). You could even check that ctx isn't NULL in match_tree_refs() to hide that code from audit_filter().

> + break; > default: > goto unlock_and_return; > } > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index ceb1c4596c51..0d00b9354886 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -125,11 +125,6 @@ struct audit_aux_data_bprm_fcaps { > struct audit_cap_data new_pcap; > }; > > -struct audit_tree_refs { > - struct audit_tree_refs *next; > - struct audit_chunk *c[31]; > -}; > - > static int audit_match_perm(struct audit_context *ctx, int mask) > { > unsigned n; > @@ -286,29 +281,6 @@ static void free_tree_refs(struct audit_context *ctx) > } > } > > -static int match_tree_refs(struct audit_context *ctx, struct audit_tree *tree) > -{ > -#ifdef CONFIG_AUDIT_TREE > - struct audit_tree_refs *p; > - int n; > - if (!tree) > - return 0; > - /* full ones */ > - for (p = ctx->first_trees; p != ctx->trees; p = p->next) { > - for (n = 0; n < 31; n++) > - if (audit_tree_match(p->c[n], tree)) > - return 1; > - } > - /* partial */ > - if (p) { > - for (n = ctx->tree_count; n < 31; n++) > - if (audit_tree_match(p->c[n], tree)) > - return 1; > - } > -#endif > - return 0; > -} > - Why did you move match_tree_refs() out of auditsc.c?

> static int audit_compare_uid(kuid_t uid, > struct audit_names *name, > struct audit_field *f, > -- > 2.17.0 > - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635