This patch removes the restriction of the AUDIT_EXE field to only SYSCALL filter and teaches audit_filter to recognize this field. This makes it possible to write rule lists such as: auditctl -a exit,always [some general rule] # Filter out events with executable name /bin/exe1 or /bin/exe2: auditctl -a exclude,always -F exe=/bin/exe1 auditctl -a exclude,always -F exe=/bin/exe2 See: https://github.com/linux-audit/audit-kernel/issues/54 Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com&gt; --- kernel/auditfilter.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index eaa320148d97..6db9847ca031 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -428,8 +428,6 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f) case AUDIT_EXE: if (f->op != Audit_not_equal && f->op != Audit_equal) return -EINVAL; - if (entry->rule.listnr != AUDIT_FILTER_EXIT) - return -EINVAL; break; } return 0; @@ -1360,6 +1358,11 @@ int audit_filter(int msgtype, unsigned int listtype) f->type, f->op, f->lsm_rule, NULL); } break; + case AUDIT_EXE: + result = audit_exe_compare(current, e->rule.exe); + if (f->op == Audit_not_equal) + result = !result; + break; default: goto unlock_and_return; } -- 2.17.0

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);

+ 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; -} -

static int audit_compare_uid(kuid_t uid, struct audit_names *name, struct audit_field *f, -- 2.17.0

2018-05-31 22:52 GMT+02:00 Richard Guy Briggs <rgb(a)redhat.com&gt;: > 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(). I wasn't sure if I could do that, since audit_filter didn't need the context until now and it is called from two places: audit_log_start -- which accepts context as an argument (doesn't get it from task, and it can be called with ctx == NULL).

audit_receive_msg -- this function doesn't work with context at all, so I wasn't sure if audit_filter should consider it being NULL or if it should get it from the current task. My hunch is the second option is the right one, but the first one is safer (AUDIT_DIR will simply never be checked here). I don't have such insight into the logic of audit_context's lifetime, so I need someone to tell me what makes more sense here.

>> + 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? Because now it can be called from both 'audit_filter_rules()' (defined in auditsc.c) and 'audit_filter()' (defined in auditfilter.c).

I'm actually playing with the idea of unifying the filtering logic in these two functions, where sharing this function wouldn't be necessary. However, that is quite a big change (a lot of LOC being moved around) so I'd prefer the simple & dirty approach now and keep the cleanup for a later patch.

Thanks for the comments! >> 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 -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.

2018-06-01 22:05 GMT+02:00 Richard Guy Briggs <rgb(a)redhat.com&gt;: > On 2018-06-01 10:12, Ondrej Mosnacek wrote: >> 2018-05-31 22:52 GMT+02:00 Richard Guy Briggs <rgb(a)redhat.com&gt;: >> > 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(). >> >> I wasn't sure if I could do that, since audit_filter didn't need the >> context until now and it is called from two places: >> >> audit_log_start -- which accepts context as an argument (doesn't get >> it from task, and it can be called with ctx == NULL). > > Alright, if current has no context, then match_tree_refs() could test > for a NULL context and return since there are no trees to check. Having > said that, there are cases where audit_log_start() is deliberately > handed a NULL context which should be honoured. It appears we don't > have a choice but to pass in the context. OK, so I'll leave that part as it is. > >> audit_receive_msg -- this function doesn't work with context at all, >> so I wasn't sure if audit_filter should consider it being NULL or if >> it should get it from the current task. My hunch is the second option >> is the right one, but the first one is safer (AUDIT_DIR will simply >> never be checked here). I don't have such insight into the logic of >> audit_context's lifetime, so I need someone to tell me what makes more >> sense here. > > That is starting to work with context. The recent FEATURE_CHANGE patch > to connect records of the same event uses current->audit_context (now > audit_context()) from audit_log_feature_change() called from > audit_set_feature() called from audit_receive_msg(). > > There is also a work in progress to convert all the CONFIG_CHANGE > records over. I'm just trying to track down all the instances. OK, so does that mean that I should just pass audit_context() instead of NULL in audit_receive_msg()?

The part that calls audit_filter() only deals with user messages (AUDIT_USER/AUDIT_FIRST..LAST_USER_MESSAGE) -- does it make sense to use the task's context in AUDIT_FILTER_USER (right now just with AUDIT_DIR)? It already filters based on the task's credentials, so perhaps it does, but I don't know...

>> >> + 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? >> >> Because now it can be called from both 'audit_filter_rules()' (defined >> in auditsc.c) and 'audit_filter()' (defined in auditfilter.c). > > So since kernel/audit.h is included in all kernel/audit*.c files, having > the prototype in there from any kernel/audit*.c file should make the > function available to all functions in kernel/audit*.c files. You're right, I could have just added a declaration and export the function from auditsc.c... I will need to add a fallback definition for when CONFIG_AUDITSYSCALL is not enabled, but in that case tasks never have audit_context set anyway, so it's fine. I will just export the function in the next version. > >> I'm actually playing with the idea of unifying the filtering logic in >> these two functions, where sharing this function wouldn't be >> necessary. However, that is quite a big change (a lot of LOC being >> moved around) so I'd prefer the simple & dirty approach now and keep >> the cleanup for a later patch. > > Some of the filtering has already been refactored and merged. More > wouldn't hurt. > >> Thanks for the comments! >> >> >> 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 >> >> -- >> Ondrej Mosnacek <omosnace at redhat dot com> >> Associate Software Engineer, Security Technologies >> Red Hat, Inc. > > - 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 -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.