Linux-audit Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Moore <pmoore@redhat.com>
To: linux-audit@redhat.com
Subject: [RFC PATCH 3/4] audit: store the auditd PID as a pid struct instead of pid_t
Date: Tue, 21 Mar 2017 14:59:05 -0400	[thread overview]
Message-ID: <149012274571.14936.3505535245970325863.stgit@sifl> (raw)
In-Reply-To: <149012253566.14936.15800724055046569015.stgit@sifl>

From: Paul Moore <paul@paul-moore.com>

This is arguably the right thing to do, and will make it easier when
we start supporting multiple audit daemons in different namespaces.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 kernel/audit.c |   84 ++++++++++++++++++++++++++++++++++++++------------------
 kernel/audit.h |    2 +
 2 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 6cbf47a372e8..b718bf3a73f8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -58,6 +58,7 @@
 #include <linux/rcupdate.h>
 #include <linux/mutex.h>
 #include <linux/gfp.h>
+#include <linux/pid.h>
 
 #include <linux/audit.h>
 
@@ -117,7 +118,7 @@ struct audit_net {
  * or the included spinlock for writing.
  */
 static struct auditd_connection {
-	int pid;
+	struct pid *pid;
 	u32 portid;
 	struct net *net;
 	spinlock_t lock;
@@ -221,18 +222,41 @@ struct audit_reply {
  * Description:
  * Return 1 if the task is a registered audit daemon, 0 otherwise.
  */
-int auditd_test_task(const struct task_struct *task)
+int auditd_test_task(struct task_struct *task)
 {
 	int rc;
 
 	rcu_read_lock();
-	rc = (auditd_conn.pid && task->tgid == auditd_conn.pid ? 1 : 0);
+	rc = (auditd_conn.pid && auditd_conn.pid == task_tgid(task) ? 1 : 0);
 	rcu_read_unlock();
 
 	return rc;
 }
 
 /**
+ * auditd_pid_vnr - Return the auditd PID relative to the namespace
+ * @auditd: the auditd connection
+ *
+ * Description:
+ * Returns the PID in relation to the namespace, 0 on failure.  This function
+ * takes the RCU read lock internally, but if the caller needs to protect the
+ * auditd_connection pointer it should take the RCU read lock as well.
+ */
+static pid_t auditd_pid_vnr(const struct auditd_connection *auditd)
+{
+	pid_t pid;
+
+	rcu_read_lock();
+	if (!auditd || !auditd->pid)
+		pid = 0;
+	else
+		pid = pid_vnr(auditd->pid);
+	rcu_read_unlock();
+
+	return pid;
+}
+
+/**
  * audit_get_sk - Return the audit socket for the given network namespace
  * @net: the destination network namespace
  *
@@ -429,12 +453,17 @@ static int audit_set_failure(u32 state)
  * This function will obtain and drop network namespace references as
  * necessary.
  */
-static void auditd_set(int pid, u32 portid, struct net *net)
+static void auditd_set(struct pid *pid, u32 portid, struct net *net)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&auditd_conn.lock, flags);
-	auditd_conn.pid = pid;
+	if (auditd_conn.pid)
+		put_pid(auditd_conn.pid);
+	if (pid)
+		auditd_conn.pid = get_pid(pid);
+	else
+		auditd_conn.pid = NULL;
 	auditd_conn.portid = portid;
 	if (auditd_conn.net)
 		put_net(auditd_conn.net);
@@ -1062,11 +1091,13 @@ static int audit_set_feature(struct sk_buff *skb)
 	return 0;
 }
 
-static int audit_replace(pid_t pid)
+static int audit_replace(struct pid *pid)
 {
+	pid_t pvnr;
 	struct sk_buff *skb;
 
-	skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pid, sizeof(pid));
+	pvnr = pid_vnr(pid);
+	skb = audit_make_reply(0, AUDIT_REPLACE, 0, 0, &pvnr, sizeof(pvnr));
 	if (!skb)
 		return -ENOMEM;
 	return auditd_send_unicast_skb(skb);
@@ -1096,9 +1127,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		memset(&s, 0, sizeof(s));
 		s.enabled		= audit_enabled;
 		s.failure		= audit_failure;
-		rcu_read_lock();
-		s.pid			= auditd_conn.pid;
-		rcu_read_unlock();
+		/* NOTE: use pid_vnr() so the PID is relative to the current
+		 *       namespace */
+		s.pid			= auditd_pid_vnr(&auditd_conn);
 		s.rate_limit		= audit_rate_limit;
 		s.backlog_limit		= audit_backlog_limit;
 		s.lost			= atomic_read(&audit_lost);
@@ -1124,36 +1155,36 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 				return err;
 		}
 		if (s.mask & AUDIT_STATUS_PID) {
-			/* NOTE: we are using task_tgid_vnr() below because
-			 *       the s.pid value is relative to the namespace
-			 *       of the caller; at present this doesn't matter
-			 *       much since you can really only run auditd
-			 *       from the initial pid namespace, but something
-			 *       to keep in mind if this changes */
-			int new_pid = s.pid;
+			/* NOTE: we are using the vnr PID functions below
+			 *       because the s.pid value is relative to the
+			 *       namespace of the caller; at present this
+			 *       doesn't matter much since you can really only
+			 *       run auditd from the initial pid namespace, but
+			 *       something to keep in mind if this changes */
+			pid_t new_pid = s.pid;
 			pid_t auditd_pid;
-			pid_t requesting_pid = task_tgid_vnr(current);
+			struct pid *req_pid = task_tgid(current);
+
+			/* sanity check - PID values must match */
+			if (new_pid != pid_vnr(req_pid))
+				return -EINVAL;
 
 			/* test the auditd connection */
-			audit_replace(requesting_pid);
+			audit_replace(req_pid);
 
-			rcu_read_lock();
-			auditd_pid = auditd_conn.pid;
+			auditd_pid = auditd_pid_vnr(&auditd_conn);
 			/* only the current auditd can unregister itself */
-			if ((!new_pid) && (requesting_pid != auditd_pid)) {
-				rcu_read_unlock();
+			if ((!new_pid) && (new_pid != auditd_pid)) {
 				audit_log_config_change("audit_pid", new_pid,
 							auditd_pid, 0);
 				return -EACCES;
 			}
 			/* replacing a healthy auditd is not allowed */
 			if (auditd_pid && new_pid) {
-				rcu_read_unlock();
 				audit_log_config_change("audit_pid", new_pid,
 							auditd_pid, 0);
 				return -EEXIST;
 			}
-			rcu_read_unlock();
 
 			if (audit_enabled != AUDIT_OFF)
 				audit_log_config_change("audit_pid", new_pid,
@@ -1161,8 +1192,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 			if (new_pid) {
 				/* register a new auditd connection */
-				auditd_set(new_pid,
-					   NETLINK_CB(skb).portid,
+				auditd_set(req_pid, NETLINK_CB(skb).portid,
 					   sock_net(NETLINK_CB(skb).sk));
 				/* try to process any backlog */
 				wake_up_interruptible(&kauditd_wait);
diff --git a/kernel/audit.h b/kernel/audit.h
index c21b74dd7ff2..d9b9af769128 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -218,7 +218,7 @@ extern void audit_log_name(struct audit_context *context,
 			   struct audit_names *n, const struct path *path,
 			   int record_num, int *call_panic);
 
-extern int auditd_test_task(const struct task_struct *task);
+extern int auditd_test_task(struct task_struct *task);
 
 #define AUDIT_INODE_BUCKETS	32
 extern struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];

  parent reply	other threads:[~2017-03-21 18:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 18:58 [RFC PATCH 0/4] Assorted audit fixes/improvements Paul Moore
2017-03-21 18:58 ` [RFC PATCH 1/4] audit: combine audit_receive() and audit_receive_skb() Paul Moore
2017-04-10  3:38   ` Richard Guy Briggs
2017-04-11 19:47     ` Paul Moore
2017-03-21 18:58 ` [RFC PATCH 2/4] audit: kernel generated netlink traffic should have a portid of 0 Paul Moore
2017-04-10  3:41   ` Richard Guy Briggs
2017-04-11 19:49     ` Paul Moore
2017-03-21 18:59 ` Paul Moore [this message]
2017-04-10  4:30   ` [RFC PATCH 3/4] audit: store the auditd PID as a pid struct instead of pid_t Richard Guy Briggs
2017-04-11 19:56     ` Paul Moore
2017-03-21 18:59 ` [RFC PATCH 4/4] audit: use kmem_cache to manage the audit_buffer cache Paul Moore
2017-03-21 19:04   ` Paul Moore
2017-04-10  4:04   ` Richard Guy Briggs
2017-04-11 20:07     ` Paul Moore
2017-04-12  4:59       ` Richard Guy Briggs
2017-04-12 15:51         ` Paul Moore

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=149012274571.14936.3505535245970325863.stgit@sifl \
    --to=pmoore@redhat.com \
    --cc=linux-audit@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