Re: [PATCH V6 3/4] audit: convert audit_exe to audit_fsnotify

Instead of just hard coding the ino and dev of the executable we care about at the moment the rule is inserted into the kernel, use the new audit_fsnotify infrastructure. This means that if the inode in question is unlinked and creat'd (aka updated) the rule will just continue to work. Signed-off-by: Eric Paris <eparis(a)redhat.com&gt; RGB: Clean up exe with similar interface as watch and tree. RGB: Clean up audit exe mark just before audit_free_rule() rather than in it to avoid mutex in software interrupt context. Based-on-code-by: Eric Paris <eparis(a)redhat.com&gt; Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt;

diff --git a/include/linux/audit.h b/include/linux/audit.h index 95463a2..aee456f 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -59,7 +59,7 @@ struct audit_krule { struct audit_field *inode_f; /* quick access to an inode field */ struct audit_watch *watch; /* associated watch */ struct audit_tree *tree; /* associated watched tree */ - struct audit_exe *exe; + struct audit_fsnotify_mark *exe; struct list_head rlist; /* entry in audit_{watch,tree}.rules list */ struct list_head list; /* for AUDIT_LIST* purposes only */ u64 prio; diff --git a/kernel/audit.h b/kernel/audit.h index 491bd4b..eeaf054 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -51,7 +51,6 @@ enum audit_state { /* Rule lists */ struct audit_watch; struct audit_fsnotify_mark; -struct audit_exe; struct audit_tree; struct audit_chunk; @@ -279,11 +278,8 @@ extern void audit_remove_mark(struct audit_fsnotify_mark *audit_mark); extern void audit_remove_mark_rule(struct audit_krule *krule); extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long ino, dev_t dev); -extern int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len, u32 op); -extern void audit_remove_exe_rule(struct audit_krule *krule); -extern char *audit_exe_path(struct audit_exe *exe); extern int audit_dupe_exe(struct audit_krule *new, struct audit_krule *old); -extern int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe); +extern int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark); #else #define audit_put_watch(w) {} @@ -302,36 +298,19 @@ static inline char *audit_mark_path(struct audit_fsnotify_mark *mark) } #define audit_remove_mark(m) BUG() #define audit_remove_mark_rule(k) BUG() -static inline int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long ino, dev_t dev) -{ - BUG(); - return 0; -} -static inline int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len, u32 op) -{ - return -EINVAL; -} -static inline void audit_remove_exe_rule(struct audit_krule *krule) -{ - BUG(); - return 0; -} -static inline char *audit_exe_path(struct audit_exe *exe) +static inline int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark) { BUG(); - return ""; + return -EINVAL; } + static inline int audit_dupe_exe(struct audit_krule *new, struct audit_krule *old) { BUG(); - return -EINVAL -} -static inline int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe) -{ - BUG(); - return 0; + return -EINVAL; } + #endif /* CONFIG_AUDIT_WATCH */ #ifdef CONFIG_AUDIT_TREE diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c index d4cc8b5..75ad4f2 100644 --- a/kernel/audit_exe.c +++ b/kernel/audit_exe.c @@ -17,93 +17,30 @@ #include <linux/kernel.h> #include <linux/audit.h> -#include <linux/mutex.h> #include <linux/fs.h> #include <linux/namei.h> #include <linux/slab.h> #include "audit.h" -struct audit_exe { - char *pathname; - unsigned long ino; - dev_t dev; -}; - -/* Translate a watch string to kernel respresentation. */ -int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len, u32 op) -{ - struct audit_exe *exe; - struct path path; - struct dentry *dentry; - unsigned long ino; - dev_t dev; - - if (pathname[0] != '/' || pathname[len-1] == '/') - return -EINVAL; - - dentry = kern_path_locked(pathname, &path); - if (IS_ERR(dentry)) - return PTR_ERR(dentry); - mutex_unlock(&path.dentry->d_inode->i_mutex); - - if (!dentry->d_inode) - return -ENOENT; - dev = dentry->d_inode->i_sb->s_dev; - ino = dentry->d_inode->i_ino; - dput(dentry); - - exe = kmalloc(sizeof(*exe), GFP_KERNEL); - if (!exe) - return -ENOMEM; - exe->ino = ino; - exe->dev = dev; - exe->pathname = pathname; - krule->exe = exe; - - return 0; -} - -void audit_remove_exe_rule(struct audit_krule *krule) -{ - struct audit_exe *exe; - - exe = krule->exe; - krule->exe = NULL; - kfree(exe->pathname); - kfree(exe); -} - -char *audit_exe_path(struct audit_exe *exe) -{ - return exe->pathname; -} - int audit_dupe_exe(struct audit_krule *new, struct audit_krule *old) { - struct audit_exe *exe; - - exe = kmalloc(sizeof(*exe), GFP_KERNEL); - if (!exe) - return -ENOMEM; + struct audit_fsnotify_mark *audit_mark; + char *pathname; - exe->pathname = kstrdup(old->exe->pathname, GFP_KERNEL); - if (!exe->pathname) { - kfree(exe); - return -ENOMEM; - } + pathname = audit_mark_path(old->exe); - exe->ino = old->exe->ino; - exe->dev = old->exe->dev; - new->exe = exe; + audit_mark = audit_alloc_mark(new, pathname, strlen(pathname)); + if (IS_ERR(audit_mark)) + return PTR_ERR(audit_mark); + new->exe = audit_mark; return 0; } -int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe) +int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark *mark) { - if (tsk->mm->exe_file->f_inode->i_ino != exe->ino) - return 0; - if (tsk->mm->exe_file->f_inode->i_sb->s_dev != exe->dev) - return 0; - return 1; + unsigned long ino = tsk->mm->exe_file->f_inode->i_ino; + dev_t dev = tsk->mm->exe_file->f_inode->i_sb->s_dev; + + return audit_mark_compare(mark, ino, dev); } diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index b0f9877..94ecdab 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -479,6 +479,8 @@ static void kill_rules(struct audit_tree *tree) if (rule->tree) { /* not a half-baked one */ audit_tree_log_remove_rule(rule); + if (entry->rule.exe) + audit_remove_mark(entry->rule.exe); rule->tree = NULL; list_del_rcu(&entry->list); list_del(&entry->rule.list); diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 8f123d7..4aaa393 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -312,6 +312,8 @@ static void audit_update_watch(struct audit_parent *parent, list_replace(&oentry->rule.list, &nentry->rule.list); } + if (oentry->rule.exe) + audit_remove_mark(oentry->rule.exe); audit_watch_log_rule_change(r, owatch, "updated_rules"); @@ -342,6 +344,8 @@ static void audit_remove_parent_watches(struct audit_parent *parent) list_for_each_entry_safe(r, nextr, &w->rules, rlist) { e = container_of(r, struct audit_entry, rule); audit_watch_log_rule_change(r, w, "remove_rule"); + if (e->rule.exe) + audit_remove_mark(e->rule.exe); list_del(&r->rlist); list_del(&r->list); list_del_rcu(&e->list); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index bbb39ec..f65c97f 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -426,6 +426,7 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data, size_t remain = datasz - sizeof(struct audit_rule_data); int i; char *str; + struct audit_fsnotify_mark *audit_mark; entry = audit_to_entry_common(data); if (IS_ERR(entry)) @@ -557,11 +558,13 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data, } entry->rule.buflen += f->val; - err = audit_make_exe_rule(&entry->rule, str, f->val, f->op); - if (err) { - kfree(str); + audit_mark = audit_alloc_mark(&entry->rule, str, f->val); + kfree(str); + if (IS_ERR(audit_mark)) { + err = PTR_ERR(audit_mark); goto exit_free; } + entry->rule.exe = audit_mark; break; } } @@ -575,6 +578,8 @@ exit_nofree: exit_free: if (entry->rule.tree) audit_put_tree(entry->rule.tree); /* that's the temporary one */ + if (entry->rule.exe) + audit_remove_mark(entry->rule.exe); /* that's the template one */ audit_free_rule(entry); return ERR_PTR(err); } @@ -642,7 +647,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule) case AUDIT_EXE: case AUDIT_EXE_CHILDREN: data->buflen += data->values[i] = - audit_pack_string(&bufp, audit_exe_path(krule->exe)); + audit_pack_string(&bufp, audit_mark_path(krule->exe)); break; case AUDIT_LOGINUID_SET: if (krule->pflags & AUDIT_LOGINUID_LEGACY && !f->val) { @@ -710,8 +715,8 @@ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b) case AUDIT_EXE: case AUDIT_EXE_CHILDREN: /* both paths exist based on above type compare */ - if (strcmp(audit_exe_path(a->exe), - audit_exe_path(b->exe))) + if (strcmp(audit_mark_path(a->exe), + audit_mark_path(b->exe))) return 1; break; case AUDIT_UID: @@ -842,6 +847,8 @@ struct audit_entry *audit_dupe_rule(struct audit_krule *old) break; } if (err) { + if (new->exe) + audit_remove_mark(new->exe); audit_free_rule(entry); return ERR_PTR(err); } @@ -1008,7 +1015,7 @@ int audit_del_rule(struct audit_entry *entry) audit_remove_tree_rule(&e->rule); if (e->rule.exe) - audit_remove_exe_rule(&e->rule); + audit_remove_mark_rule(&e->rule); list_del_rcu(&e->list); list_del(&e->rule.list); @@ -1113,8 +1120,11 @@ int audit_rule_change(int type, __u32 portid, int seq, void *data, WARN_ON(1); } - if (err || type == AUDIT_DEL_RULE) + if (err || type == AUDIT_DEL_RULE) { + if (entry->rule.exe) + audit_remove_mark(entry->rule.exe); audit_free_rule(entry); + } return err; } @@ -1406,6 +1416,8 @@ static int update_lsm_rule(struct audit_krule *r) return 0; nentry = audit_dupe_rule(r); + if (entry->rule.exe) + audit_remove_mark(entry->rule.exe); if (IS_ERR(nentry)) { /* save the first error encountered for the * return value */