linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: linux-audit@redhat.com, linux-kernel@vger.kernel.org
Cc: Richard Guy Briggs <rgb@redhat.com>,
	sgrubb@redhat.com, pmoore@redhat.com, eparis@redhat.com,
	pmoody@google.com
Subject: [PATCH V6 3/4] audit: convert audit_exe to audit_fsnotify
Date: Tue, 14 Jul 2015 11:50:25 -0400	[thread overview]
Message-ID: <2b37c4f1602bd672ee5b241c0fda57559d4a576a.1436823321.git.rgb@redhat.com> (raw)
In-Reply-To: <cover.1436823321.git.rgb@redhat.com>
In-Reply-To: <cover.1436823321.git.rgb@redhat.com>

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


  parent reply	other threads:[~2015-07-14 15:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-14 15:50 [PATCH V6 0/4] audit by executable name Richard Guy Briggs
2015-07-14 15:50 ` [PATCH V6 1/4] audit: implement audit by executable Richard Guy Briggs
2015-07-17  1:18   ` Paul Moore
2015-07-17 15:33     ` Richard Guy Briggs
2015-07-17 18:24       ` Paul Moore
2015-07-17 20:46         ` Richard Guy Briggs
2015-07-20 15:10           ` Paul Moore
2015-07-17 20:27       ` Richard Guy Briggs
2015-07-14 15:50 ` [PATCH V6 2/4] audit: clean simple fsnotify implementation Richard Guy Briggs
2015-07-17  1:45   ` Paul Moore
2015-08-01 20:03     ` Richard Guy Briggs
2015-07-14 15:50 ` Richard Guy Briggs [this message]
2015-07-17  1:54   ` [PATCH V6 3/4] audit: convert audit_exe to audit_fsnotify Paul Moore
2015-07-17  2:02     ` Richard Guy Briggs
2015-07-14 15:50 ` [PATCH V6 4/4] audit: avoid double copying the audit_exe path string Richard Guy Briggs
2015-07-17  1:56   ` Paul Moore
2015-07-17  2:01     ` Richard Guy Briggs
2015-07-17  2:42       ` Paul Moore
2015-07-17  3:01       ` Eric Paris
2015-07-17  3:24         ` Paul Moore
2015-07-17 16:48           ` Richard Guy Briggs
2015-07-17 18:09             ` Paul Moore
2015-07-17 16:18         ` Richard Guy Briggs
2015-07-17 18:01           ` Paul Moore
2015-07-15 12:28 ` [PATCH V6 0/4] audit by executable name Steve Grubb
2015-07-15 18:23   ` Richard Guy Briggs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2b37c4f1602bd672ee5b241c0fda57559d4a576a.1436823321.git.rgb@redhat.com \
    --to=rgb@redhat.com \
    --cc=eparis@redhat.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmoody@google.com \
    --cc=pmoore@redhat.com \
    --cc=sgrubb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).