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

Richard Guy Briggs
2015-07-14T15:51:44Z

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@redhat.com>

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@redhat.com>
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
include/linux/audit.h | 2 +-
kernel/audit.h | 33 +++---------------
kernel/audit_exe.c | 87 +++++++------------------------------------------
kernel/audit_tree.c | 2 +
kernel/audit_watch.c | 4 ++
kernel/auditfilter.c | 28 +++++++++++----
6 files changed, 45 insertions(+), 111 deletions(-)

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 */
--
1.7.1