Discussion:
[PATCH 00/12] RFC: steps to make audit pid namespace-safe
(too old to reply)
Richard Guy Briggs
2013-08-20 21:31:52 UTC
Permalink
This patchset is a revival of some of Eric Biederman's work to make audit
pid-namespace-safe.

In a couple of places, audit was printing PIDs in the task's pid namespace
rather than relative to the audit daemon's pid namespace, which currently is
init_pid_ns.

It also allows processes to log audit user messages in their own pid
namespaces, which was not previously permitted. Please see:
https://bugzilla.redhat.com/show_bug.cgi?id=947530
https://bugs.launchpad.net/ubuntu/+source/vsftpd/+bug/1160372
https://bugzilla.novell.com/show_bug.cgi?id=786024

Part of the cleanup here involves deprecating task->pid and task->tgid, which
are error-prone duplicates of the task->pids structure

The next step which I hope to add to this patchset will be to purge task->pid
and task->tgid from the rest of the kernel if possible. Once that is done,
task_pid_nr_init_ns() and task_tgid_nr_init_ns() that were introduced in patch
05/12 and used in patches 06/12 and 08/12 could be replaced with task_pid_nr()
and task_tgid_nr(). Eric B. did take a stab at that, but checking all the
subtleties will be non-trivial.

Does anyone have any opinions or better yet hard data on cache line misses
between pid_nr(struct pid*) and pid_nr_ns(struct pid*, &init_pid_ns)? I'd
like to see pid_nr() use pid_nr_ns(struct pid*, &init_pid_ns), or
pid_nr_init_ns() eliminated in favour of the original pid_nr(). pid_nr()
currently accesses the first level of the pid structure without having to
dereference the level number. If there is an actual speed difference, it could
be worth keeping, otherwise, I'd prefer to simplify that code.

Eric also had a patch to add a printk option to format a struct pid pointer
which was PID namespace-aware. I don't see the point, but I'll let him explain
it.

Discuss.

Eric W. Biederman (5):
audit: Kill the unused struct audit_aux_data_capset
audit: Simplify and correct audit_log_capset

Richard Guy Briggs (7):
audit: fix netlink portid naming and types
pid: get ppid pid_t of task in init_pid_ns safely
audit: convert PPIDs to the inital PID namespace.
pid: get pid_t of task in init_pid_ns correctly
audit: store audit_pid as a struct pid pointer
audit: anchor all pid references in the initial pid namespace
pid: modify task_pid_nr to work without task->pid.
pid: modify task_tgid_nr to work without task->tgid.
pid: rewrite task helper functions avoiding task->pid and task->tgid
pid: mark struct task const in helper functions

drivers/tty/tty_audit.c | 3 +-
include/linux/audit.h | 8 ++--
include/linux/pid.h | 6 +++
include/linux/sched.h | 81 ++++++++++++++++++++++++----------
kernel/audit.c | 76 +++++++++++++++++++------------
kernel/audit.h | 12 +++---
kernel/auditfilter.c | 35 +++++++++++----
kernel/auditsc.c | 36 ++++++---------
kernel/capability.c | 2 +-
kernel/pid.c | 4 +-
security/apparmor/audit.c | 7 +--
security/integrity/integrity_audit.c | 2 +-
security/lsm_audit.c | 11 +++--
security/tomoyo/audit.c | 2 +-
14 files changed, 177 insertions(+), 108 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2013-08-20 21:31:53 UTC
Permalink
From: Eric W. Biederman <***@xmission.com>

Signed-off-by: "Eric W. Biederman" <***@xmission.com>
(cherry picked from commit 6904431d6b41190e42d6b94430b67cb4e7e6a4b7)
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/auditsc.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9845cb3..8c644c5 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -121,12 +121,6 @@ struct audit_aux_data_bprm_fcaps {
struct audit_cap_data new_pcap;
};

-struct audit_aux_data_capset {
- struct audit_aux_data d;
- pid_t pid;
- struct audit_cap_data cap;
-};
-
struct audit_tree_refs {
struct audit_tree_refs *next;
struct audit_chunk *c[31];
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2013-08-20 21:31:55 UTC
Permalink
Added the functions
task_ppid()
task_ppid_nr_ns()
task_ppid_nr_init_ns()
to safely abstract the lookup of the PPID (real_parent's tgid) of a process,
including rcu locking, in any required pid namespace. This provides an
alternative to sys_getppid(), which is relative to the child process' pid
namespace.

(informed by ebiederman's 6c621b7e)
Cc: ***@vger.kernel.org
Cc: Eric W. Biederman <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/sched.h | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d722490..ec04090 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1448,6 +1448,11 @@ static inline struct pid *task_session(struct task_struct *task)
return task->group_leader->pids[PIDTYPE_SID].pid;
}

+static inline struct pid *task_ppid(struct task_struct *task)
+{
+ return task_tgid(rcu_dereference(current->real_parent));
+}
+
struct pid_namespace;

/*
@@ -1496,6 +1501,24 @@ static inline pid_t task_tgid_vnr(struct task_struct *tsk)
}


+static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ pid_t pid;
+
+ rcu_read_lock();
+ pid = pid_nr_ns(task_ppid(current), ns);
+ rcu_read_unlock();
+
+ return pid;
+}
+
+static inline pid_t task_ppid_nr_init_ns(struct task_struct *tsk)
+{
+ return task_ppid_nr_ns(tsk, &init_pid_ns);
+}
+
+
static inline pid_t task_pgrp_nr_ns(struct task_struct *tsk,
struct pid_namespace *ns)
{
--
1.7.1
Oleg Nesterov
2013-08-27 17:21:55 UTC
Permalink
Post by Richard Guy Briggs
Added the functions
task_ppid()
task_ppid_nr_ns()
task_ppid_nr_init_ns()
to safely abstract the lookup of the PPID
but it is not safe.
Post by Richard Guy Briggs
+static inline struct pid *task_ppid(struct task_struct *task)
+{
+ return task_tgid(rcu_dereference(current->real_parent));
^^^^^^^
task?
Post by Richard Guy Briggs
+static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
+ struct pid_namespace *ns)
+{
+ pid_t pid;
+
+ rcu_read_lock();
+ pid = pid_nr_ns(task_ppid(current), ns);
^^^^^^^
again.
Post by Richard Guy Briggs
+ rcu_read_unlock();
And why this is safe?

rcu_read_lock() can't help if tsk was already dead _before_ it takes
the rcu lock. ->real_parent can point the already freed/reused/unmapped
memory.

This is safe if, for example, the caller alredy holds rcu_read_lock()
and tsk was found by find_task_by*(), or tsk is current.

Richard, just in case... I am going to vacation, I will be completely
offline till Sep 10.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2013-08-20 21:32:01 UTC
Permalink
task->pid is an error prone construct and results in duplicate maintenance.
Start it's demise by modifying task_pid_nr to not use it.

(informed by ebiederman's 3a2e8c59)
Cc: "Eric W. Biederman" <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/sched.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 9651d95..fb09b93 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1473,7 +1473,7 @@ pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type,

static inline pid_t task_pid_nr(struct task_struct *tsk)
{
- return tsk->pid;
+ return pid_nr(task_pid(tsk));
}

static inline pid_t task_pid_nr_ns(struct task_struct *tsk,
--
1.7.1
Richard Guy Briggs
2013-08-20 21:31:54 UTC
Permalink
Normally, netlink ports use the PID of the userspace process as the port ID.
If the PID is already in use by a port, the kernel will allocate another port
ID to avoid conflict. Re-name all references to netlink ports from pid to
portid to reflect this reality and avoid confusion with actual PIDs. Ports
use the __u32 type, so re-type all portids accordingly.

(This patch is very similar to ebiederman's 5deadd69)

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/audit.h | 2 +-
kernel/audit.c | 32 ++++++++++++++++----------------
kernel/audit.h | 8 ++++----
kernel/auditfilter.c | 18 ++++++++++--------
4 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 729a4d1..a3af0fa 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -462,7 +462,7 @@ extern int audit_update_lsm_rules(void);
/* Private API (for audit.c only) */
extern int audit_filter_user(int type);
extern int audit_filter_type(int type);
-extern int audit_receive_filter(int type, int pid, int seq,
+extern int audit_receive_filter(int type, __u32 portid, int seq,
void *data, size_t datasz);
extern int audit_enabled;
#else /* CONFIG_AUDIT */
diff --git a/kernel/audit.c b/kernel/audit.c
index 91e53d0..2476334 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -94,7 +94,7 @@ static int audit_failure = AUDIT_FAIL_PRINTK;
* the portid to use to send netlink messages to that process.
*/
int audit_pid;
-static int audit_nlk_portid;
+static __u32 audit_nlk_portid;

/* If audit_rate_limit is non-zero, limit the rate of sending audit records
* to that number per second. This prevents DoS attacks, but results in
@@ -165,15 +165,15 @@ struct audit_buffer {
};

struct audit_reply {
- int pid;
+ __u32 portid;
struct sk_buff *skb;
};

-static void audit_set_pid(struct audit_buffer *ab, pid_t pid)
+static void audit_set_portid(struct audit_buffer *ab, __u32 portid)
{
if (ab) {
struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
- nlh->nlmsg_pid = pid;
+ nlh->nlmsg_pid = portid;
}
}

@@ -472,7 +472,7 @@ static int kauditd_thread(void *dummy)
int audit_send_list(void *_dest)
{
struct audit_netlink_list *dest = _dest;
- int pid = dest->pid;
+ __u32 portid = dest->portid;
struct sk_buff *skb;

/* wait for parent to finish and send an ACK */
@@ -480,14 +480,14 @@ int audit_send_list(void *_dest)
mutex_unlock(&audit_cmd_mutex);

while ((skb = __skb_dequeue(&dest->q)) != NULL)
- netlink_unicast(audit_sock, skb, pid, 0);
+ netlink_unicast(audit_sock, skb, portid, 0);

kfree(dest);

return 0;
}

-struct sk_buff *audit_make_reply(int pid, int seq, int type, int done,
+struct sk_buff *audit_make_reply(__u32 portid, int seq, int type, int done,
int multi, const void *payload, int size)
{
struct sk_buff *skb;
@@ -500,7 +500,7 @@ struct sk_buff *audit_make_reply(int pid, int seq, int type, int done,
if (!skb)
return NULL;

- nlh = nlmsg_put(skb, pid, seq, t, size, flags);
+ nlh = nlmsg_put(skb, portid, seq, t, size, flags);
if (!nlh)
goto out_kfree_skb;
data = nlmsg_data(nlh);
@@ -521,13 +521,13 @@ static int audit_send_reply_thread(void *arg)

/* Ignore failure. It'll only happen if the sender goes away,
because our timeout is set to infinite. */
- netlink_unicast(audit_sock, reply->skb, reply->pid, 0);
+ netlink_unicast(audit_sock, reply->skb, reply->portid, 0);
kfree(reply);
return 0;
}
/**
* audit_send_reply - send an audit reply message via netlink
- * @pid: process id to send reply to
+ * @portid: netlink port to which to send reply
* @seq: sequence number
* @type: audit message type
* @done: done (last) flag
@@ -535,11 +535,11 @@ static int audit_send_reply_thread(void *arg)
* @payload: payload data
* @size: payload size
*
- * Allocates an skb, builds the netlink message, and sends it to the pid.
+ * Allocates an skb, builds the netlink message, and sends it to the port id.
* No failure notifications.
*/
-static void audit_send_reply(int pid, int seq, int type, int done, int multi,
- const void *payload, int size)
+static void audit_send_reply(__u32 portid, int seq, int type, int done,
+ int multi, const void *payload, int size)
{
struct sk_buff *skb;
struct task_struct *tsk;
@@ -549,11 +549,11 @@ static void audit_send_reply(int pid, int seq, int type, int done, int multi,
if (!reply)
return;

- skb = audit_make_reply(pid, seq, type, done, multi, payload, size);
+ skb = audit_make_reply(portid, seq, type, done, multi, payload, size);
if (!skb)
goto out;

- reply->pid = pid;
+ reply->portid = portid;
reply->skb = skb;

tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply");
@@ -727,7 +727,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
size--;
audit_log_n_untrustedstring(ab, data, size);
}
- audit_set_pid(ab, NETLINK_CB(skb).portid);
+ audit_set_portid(ab, NETLINK_CB(skb).portid);
audit_log_end(ab);
}
break;
diff --git a/kernel/audit.h b/kernel/audit.h
index 123c9b7..36edcf5 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -237,13 +237,13 @@ extern int audit_uid_comparator(kuid_t left, u32 op, kuid_t right);
extern int audit_gid_comparator(kgid_t left, u32 op, kgid_t right);
extern int parent_len(const char *path);
extern int audit_compare_dname_path(const char *dname, const char *path, int plen);
-extern struct sk_buff * audit_make_reply(int pid, int seq, int type,
- int done, int multi,
- const void *payload, int size);
+extern struct sk_buff *audit_make_reply(__u32 portid, int seq, int type,
+ int done, int multi,
+ const void *payload, int size);
extern void audit_panic(const char *message);

struct audit_netlink_list {
- int pid;
+ __u32 portid;
struct sk_buff_head q;
};

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index f7aee8b..381d3de 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -971,7 +971,7 @@ out:
}

/* List rules using struct audit_rule_data. */
-static void audit_list_rules(int pid, int seq, struct sk_buff_head *q)
+static void audit_list_rules(__u32 portid, int seq, struct sk_buff_head *q)
{
struct sk_buff *skb;
struct audit_krule *r;
@@ -986,14 +986,15 @@ static void audit_list_rules(int pid, int seq, struct sk_buff_head *q)
data = audit_krule_to_data(r);
if (unlikely(!data))
break;
- skb = audit_make_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data) + data->buflen);
+ skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES,
+ 0, 1, data,
+ sizeof(*data) + data->buflen);
if (skb)
skb_queue_tail(q, skb);
kfree(data);
}
}
- skb = audit_make_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
+ skb = audit_make_reply(portid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
if (skb)
skb_queue_tail(q, skb);
}
@@ -1023,12 +1024,13 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
/**
* audit_receive_filter - apply all rules to the specified message type
* @type: audit message type
- * @pid: target pid for netlink audit messages
+ * @portid: target port id for netlink audit messages
* @seq: netlink audit message sequence (serial) number
* @data: payload data
* @datasz: size of payload data
*/
-int audit_receive_filter(int type, int pid, int seq, void *data, size_t datasz)
+int audit_receive_filter(int type, __u32 portid, int seq, void *data,
+ size_t datasz)
{
struct task_struct *tsk;
struct audit_netlink_list *dest;
@@ -1046,11 +1048,11 @@ int audit_receive_filter(int type, int pid, int seq, void *data, size_t datasz)
dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL);
if (!dest)
return -ENOMEM;
- dest->pid = pid;
+ dest->portid = portid;
skb_queue_head_init(&dest->q);

mutex_lock(&audit_filter_mutex);
- audit_list_rules(pid, seq, &dest->q);
+ audit_list_rules(portid, seq, &dest->q);
mutex_unlock(&audit_filter_mutex);

tsk = kthread_run(audit_send_list, dest, "audit_send_list");
--
1.7.1
Richard Guy Briggs
2013-08-20 21:32:04 UTC
Permalink
It doesn't make any sense to recallers to pass in a non-const struct
task so update the function signatures to only require a const struct
task.

(informed by ebiederman's c76b2526)
Cc: "Eric W. Biederman" <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/sched.h | 44 ++++++++++++++++++++++----------------------
kernel/pid.c | 4 ++--
2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 46e739d..a585b51 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1423,12 +1423,12 @@ static inline void set_numabalancing_state(bool enabled)
}
#endif

-static inline struct pid *task_pid(struct task_struct *task)
+static inline struct pid *task_pid(const struct task_struct *task)
{
return task->pids[PIDTYPE_PID].pid;
}

-static inline struct pid *task_tgid(struct task_struct *task)
+static inline struct pid *task_tgid(const struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_PID].pid;
}
@@ -1438,12 +1438,12 @@ static inline struct pid *task_tgid(struct task_struct *task)
* the result of task_pgrp/task_session even if task == current,
* we can race with another thread doing sys_setsid/sys_setpgid.
*/
-static inline struct pid *task_pgrp(struct task_struct *task)
+static inline struct pid *task_pgrp(const struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_PGID].pid;
}

-static inline struct pid *task_session(struct task_struct *task)
+static inline struct pid *task_session(const struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_SID].pid;
}
@@ -1468,50 +1468,50 @@ struct pid_namespace;
*
* see also pid_nr() etc in include/linux/pid.h
*/
-pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type,
+pid_t __task_pid_nr_ns(const struct task_struct *task, enum pid_type type,
struct pid_namespace *ns);

-static inline pid_t task_pid_nr(struct task_struct *tsk)
+static inline pid_t task_pid_nr(const struct task_struct *tsk)
{
return pid_nr(task_pid(tsk));
}

-static inline pid_t task_pid_nr_ns(struct task_struct *tsk,
+static inline pid_t task_pid_nr_ns(const struct task_struct *tsk,
struct pid_namespace *ns)
{
return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns);
}

-static inline pid_t task_pid_nr_init_ns(struct task_struct *tsk)
+static inline pid_t task_pid_nr_init_ns(const struct task_struct *tsk)
{
return task_pid_nr_ns(tsk, &init_pid_ns);
}

-static inline pid_t task_pid_vnr(struct task_struct *tsk)
+static inline pid_t task_pid_vnr(const struct task_struct *tsk)
{
return __task_pid_nr_ns(tsk, PIDTYPE_PID, NULL);
}


-static inline pid_t task_tgid_nr(struct task_struct *tsk)
+static inline pid_t task_tgid_nr(const struct task_struct *tsk)
{
return pid_nr(task_tgid(tsk));
}

-pid_t task_tgid_nr_ns(struct task_struct *tsk, struct pid_namespace *ns);
+pid_t task_tgid_nr_ns(const struct task_struct *tsk, struct pid_namespace *ns);

-static inline pid_t task_tgid_nr_init_ns(struct task_struct *tsk)
+static inline pid_t task_tgid_nr_init_ns(const struct task_struct *tsk)
{
return task_tgid_nr_ns(tsk, &init_pid_ns);
}

-static inline pid_t task_tgid_vnr(struct task_struct *tsk)
+static inline pid_t task_tgid_vnr(const struct task_struct *tsk)
{
return pid_vnr(task_tgid(tsk));
}


-static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
+static inline pid_t task_ppid_nr_ns(const struct task_struct *tsk,
struct pid_namespace *ns)
{
pid_t pid;
@@ -1523,37 +1523,37 @@ static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
return pid;
}

-static inline pid_t task_ppid_nr_init_ns(struct task_struct *tsk)
+static inline pid_t task_ppid_nr_init_ns(const struct task_struct *tsk)
{
return task_ppid_nr_ns(tsk, &init_pid_ns);
}


-static inline pid_t task_pgrp_nr_ns(struct task_struct *tsk,
+static inline pid_t task_pgrp_nr_ns(const struct task_struct *tsk,
struct pid_namespace *ns)
{
return __task_pid_nr_ns(tsk, PIDTYPE_PGID, ns);
}

-static inline pid_t task_pgrp_vnr(struct task_struct *tsk)
+static inline pid_t task_pgrp_vnr(const struct task_struct *tsk)
{
return __task_pid_nr_ns(tsk, PIDTYPE_PGID, NULL);
}


-static inline pid_t task_session_nr_ns(struct task_struct *tsk,
+static inline pid_t task_session_nr_ns(const struct task_struct *tsk,
struct pid_namespace *ns)
{
return __task_pid_nr_ns(tsk, PIDTYPE_SID, ns);
}

-static inline pid_t task_session_vnr(struct task_struct *tsk)
+static inline pid_t task_session_vnr(const struct task_struct *tsk)
{
return __task_pid_nr_ns(tsk, PIDTYPE_SID, NULL);
}

/* obsolete, do not use */
-static inline pid_t task_pgrp_nr(struct task_struct *tsk)
+static inline pid_t task_pgrp_nr(const struct task_struct *tsk)
{
return task_pgrp_nr_ns(tsk, &init_pid_ns);
}
@@ -1566,7 +1566,7 @@ static inline pid_t task_pgrp_nr(struct task_struct *tsk)
* If pid_alive fails, then pointers within the task structure
* can be stale and must not be dereferenced.
*/
-static inline int pid_alive(struct task_struct *p)
+static inline int pid_alive(const struct task_struct *p)
{
return p->pids[PIDTYPE_PID].pid != NULL;
}
@@ -1577,7 +1577,7 @@ static inline int pid_alive(struct task_struct *p)
*
* Check if a task structure is the first user space task the kernel created.
*/
-static inline int is_global_init(struct task_struct *tsk)
+static inline int is_global_init(const struct task_struct *tsk)
{
return task_pid_nr(tsk) == 1;
}
diff --git a/kernel/pid.c b/kernel/pid.c
index 66505c1..17755ae 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -506,7 +506,7 @@ pid_t pid_vnr(struct pid *pid)
}
EXPORT_SYMBOL_GPL(pid_vnr);

-pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type,
+pid_t __task_pid_nr_ns(const struct task_struct *task, enum pid_type type,
struct pid_namespace *ns)
{
pid_t nr = 0;
@@ -525,7 +525,7 @@ pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type,
}
EXPORT_SYMBOL(__task_pid_nr_ns);

-pid_t task_tgid_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
+pid_t task_tgid_nr_ns(const struct task_struct *tsk, struct pid_namespace *ns)
{
return pid_nr_ns(task_tgid(tsk), ns);
}
--
1.7.1
Richard Guy Briggs
2013-08-20 21:32:00 UTC
Permalink
Store and log all PIDs with reference to the initial PID namespace and
deprecate the use of the error-prone duplicity of task->pid and task->tgid.

Still only permit the audit logging daemon and control to operate from the
initial PID namespace, but allow processes to log from another PID namespace.

Cc: "Eric W. Biederman" <***@xmission.com>
(informed by ebiederman's c776b5d2)

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
drivers/tty/tty_audit.c | 3 ++-
kernel/audit.c | 15 ++++++++++-----
kernel/auditfilter.c | 17 ++++++++++++++++-
kernel/auditsc.c | 16 +++++++++-------
security/apparmor/audit.c | 2 +-
security/integrity/integrity_audit.c | 2 +-
security/lsm_audit.c | 11 +++++++----
security/tomoyo/audit.c | 2 +-
8 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index a4fdce7..a0ae01f 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -65,6 +65,7 @@ static void tty_audit_log(const char *description, int major, int minor,
{
struct audit_buffer *ab;
struct task_struct *tsk = current;
+ pid_t pid = task_pid_nr_init_ns(tsk);
uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
u32 sessionid = audit_get_sessionid(tsk);
@@ -74,7 +75,7 @@ static void tty_audit_log(const char *description, int major, int minor,
char name[sizeof(tsk->comm)];

audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d"
- " minor=%d comm=", description, tsk->pid, uid,
+ " minor=%d comm=", description, pid, uid,
loginuid, sessionid, major, minor);
get_task_comm(name, tsk);
audit_log_untrustedstring(ab, name);
diff --git a/kernel/audit.c b/kernel/audit.c
index fa1d5f5..8e4958b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -574,9 +574,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
{
int err = 0;

- /* Only support the initial namespaces for now. */
- if ((current_user_ns() != &init_user_ns) ||
- (task_active_pid_ns(current) != &init_pid_ns))
+ /* Only support initial user namespace for now. */
+ if ((current_user_ns() != &init_user_ns))
return -EPERM;

switch (msg_type) {
@@ -594,6 +593,11 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
case AUDIT_TTY_SET:
case AUDIT_TRIM:
case AUDIT_MAKE_EQUIV:
+ /* Only support auditd and auditctl in initial pid namespace
+ * for now. */
+ if ((task_active_pid_ns(current) != &init_pid_ns))
+ return -EPERM;
+
if (!capable(CAP_AUDIT_CONTROL))
err = -EPERM;
break;
@@ -614,6 +618,7 @@ static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
{
int rc = 0;
uid_t uid = from_kuid(&init_user_ns, current_uid());
+ pid_t pid = task_tgid_nr_init_ns(current);

if (!audit_enabled) {
*ab = NULL;
@@ -623,7 +628,7 @@ static int audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
*ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
if (unlikely(!*ab))
return rc;
- audit_log_format(*ab, "pid=%d uid=%u", task_tgid_vnr(current), uid);
+ audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
audit_log_session_info(*ab);
audit_log_task_context(*ab);

@@ -1605,7 +1610,7 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
" euid=%u suid=%u fsuid=%u"
" egid=%u sgid=%u fsgid=%u ses=%u tty=%s",
task_ppid_nr_init_ns(tsk),
- tsk->pid,
+ task_pid_nr_init_ns(tsk),
from_kuid(&init_user_ns, audit_get_loginuid(tsk)),
from_kuid(&init_user_ns, cred->uid),
from_kgid(&init_user_ns, cred->gid),
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 381d3de..2a60455 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -428,6 +428,19 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
f->val = 0;
}

+ if ((f->type == AUDIT_PID) || (f->type == AUDIT_PPID)) {
+ struct pid *pid;
+ rcu_read_lock();
+ pid = find_vpid(f->val);
+ if (!pid) {
+ rcu_read_unlock();
+ err = -ESRCH;
+ goto exit_free;
+ }
+ f->val = pid_nr_init_ns(pid);
+ rcu_read_unlock();
+ }
+
err = audit_field_valid(entry, f);
if (err)
goto exit_free;
@@ -1223,12 +1236,14 @@ static int audit_filter_user_rules(struct audit_krule *rule, int type,

for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &rule->fields[i];
+ pid_t pid;
int result = 0;
u32 sid;

switch (f->type) {
case AUDIT_PID:
- result = audit_comparator(task_pid_vnr(current), f->op, f->val);
+ pid = task_pid_nr_init_ns(current);
+ result = audit_comparator(pid, f->op, f->val);
break;
case AUDIT_UID:
result = audit_uid_comparator(current_uid(), f->op, f->uid);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 2dcf67d..5cde81c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -458,10 +458,12 @@ static int audit_filter_rules(struct task_struct *tsk,
struct audit_field *f = &rule->fields[i];
struct audit_names *n;
int result = 0;
+ pid_t pid;

switch (f->type) {
case AUDIT_PID:
- result = audit_comparator(tsk->pid, f->op, f->val);
+ pid = task_pid_nr_init_ns(tsk);
+ result = audit_comparator(pid, f->op, f->val);
break;
case AUDIT_PPID:
if (ctx) {
@@ -1989,7 +1991,7 @@ int audit_set_loginuid(kuid_t loginuid)
audit_log_format(ab, "login pid=%d uid=%u "
"old auid=%u new auid=%u"
" old ses=%u new ses=%u",
- task->pid,
+ task_pid_nr_init_ns(task),
from_kuid(&init_user_ns, task_uid(task)),
from_kuid(&init_user_ns, task->loginuid),
from_kuid(&init_user_ns, loginuid),
@@ -2197,7 +2199,7 @@ void __audit_ptrace(struct task_struct *t)
{
struct audit_context *context = current->audit_context;

- context->target_pid = t->pid;
+ context->target_pid = task_pid_nr_init_ns(t);
context->target_auid = audit_get_loginuid(t);
context->target_uid = task_uid(t);
context->target_sessionid = audit_get_sessionid(t);
@@ -2222,7 +2224,7 @@ int __audit_signal_info(int sig, struct task_struct *t)

if (audit_pid && task_tgid(t) == audit_pid) {
if (sig == SIGTERM || sig == SIGHUP || sig == SIGUSR1 || sig == SIGUSR2) {
- audit_sig_pid = tsk->pid;
+ audit_sig_pid = task_pid_nr_init_ns(tsk);
if (uid_valid(tsk->loginuid))
audit_sig_uid = tsk->loginuid;
else
@@ -2236,7 +2238,7 @@ int __audit_signal_info(int sig, struct task_struct *t)
/* optimize the common case by putting first signal recipient directly
* in audit_context */
if (!ctx->target_pid) {
- ctx->target_pid = t->tgid;
+ ctx->target_pid = task_tgid_nr_init_ns(t);
ctx->target_auid = audit_get_loginuid(t);
ctx->target_uid = t_uid;
ctx->target_sessionid = audit_get_sessionid(t);
@@ -2257,7 +2259,7 @@ int __audit_signal_info(int sig, struct task_struct *t)
}
BUG_ON(axp->pid_count >= AUDIT_AUX_PIDS);

- axp->target_pid[axp->pid_count] = t->tgid;
+ axp->target_pid[axp->pid_count] = task_tgid_nr_init_ns(t);
axp->target_auid[axp->pid_count] = audit_get_loginuid(t);
axp->target_uid[axp->pid_count] = t_uid;
axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
@@ -2356,7 +2358,7 @@ static void audit_log_task(struct audit_buffer *ab)
from_kgid(&init_user_ns, gid),
sessionid);
audit_log_task_context(ab);
- audit_log_format(ab, " pid=%d comm=", current->pid);
+ audit_log_format(ab, " pid=%d comm=", task_pid_nr_init_ns(current));
audit_log_untrustedstring(ab, current->comm);
}

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 497b306..afe71f5 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -148,7 +148,7 @@ static void audit_pre(struct audit_buffer *ab, void *ca)
}

if (sa->aad->tsk) {
- audit_log_format(ab, " pid=%d comm=", tsk->pid);
+ audit_log_format(ab, " pid=%d comm=", task_pid_nr_init_ns(tsk));
audit_log_untrustedstring(ab, tsk->comm);
}

diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index d7efb30..1e7291a 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -39,7 +39,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,

ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno);
audit_log_format(ab, "pid=%d uid=%u auid=%u ses=%u",
- current->pid,
+ task_pid_nr_init_ns(current),
from_kuid(&init_user_ns, current_cred()->uid),
from_kuid(&init_user_ns, audit_get_loginuid(current)),
audit_get_sessionid(current));
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 8d8d97d..4f43488 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -220,7 +220,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
*/
BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2);

- audit_log_format(ab, " pid=%d comm=", tsk->pid);
+ audit_log_format(ab, " pid=%d comm=", task_pid_nr_init_ns(tsk));
audit_log_untrustedstring(ab, tsk->comm);

switch (a->type) {
@@ -278,9 +278,12 @@ static void dump_common_audit_data(struct audit_buffer *ab,
}
case LSM_AUDIT_DATA_TASK:
tsk = a->u.tsk;
- if (tsk && tsk->pid) {
- audit_log_format(ab, " pid=%d comm=", tsk->pid);
- audit_log_untrustedstring(ab, tsk->comm);
+ if (tsk) {
+ pid_t pid = task_pid_nr_init_ns(tsk);
+ if (pid) {
+ audit_log_format(ab, " pid=%d comm=", pid);
+ audit_log_untrustedstring(ab, tsk->comm);
+ }
}
break;
case LSM_AUDIT_DATA_NET:
diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c
index c1b0037..6657607 100644
--- a/security/tomoyo/audit.c
+++ b/security/tomoyo/audit.c
@@ -147,7 +147,7 @@ static inline const char *tomoyo_filetype(const umode_t mode)
static char *tomoyo_print_header(struct tomoyo_request_info *r)
{
struct tomoyo_time stamp;
- const pid_t gpid = task_pid_nr(current);
+ const pid_t gpid = task_pid_nr_init_ns(current);
struct tomoyo_obj_info *obj = r->obj;
static const int tomoyo_buffer_len = 4096;
char *buffer = kmalloc(tomoyo_buffer_len, GFP_NOFS);
--
1.7.1
Richard Guy Briggs
2013-08-20 21:31:58 UTC
Permalink
From: Eric W. Biederman <***@xmission.com>

- Always report the current process as capset now always only works on
the current process. This prevents reporting 0 or a random pid in
a random pid namespace.

- Don't bother to pass the pid as is available.

Signed-off-by: "Eric W. Biederman" <***@xmission.com>
(cherry picked from commit bcc85f0af31af123e32858069eb2ad8f39f90e67)
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/audit.h | 6 +++---
kernel/auditsc.c | 6 ++----
kernel/capability.c | 2 +-
3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index a3af0fa..49223a6 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -218,7 +218,7 @@ extern void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat);
extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
const struct cred *new,
const struct cred *old);
-extern void __audit_log_capset(pid_t pid, const struct cred *new, const struct cred *old);
+extern void __audit_log_capset(const struct cred *new, const struct cred *old);
extern void __audit_mmap_fd(int fd, int flags);

static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
@@ -284,11 +284,11 @@ static inline int audit_log_bprm_fcaps(struct linux_binprm *bprm,
return 0;
}

-static inline void audit_log_capset(pid_t pid, const struct cred *new,
+static inline void audit_log_capset(const struct cred *new,
const struct cred *old)
{
if (unlikely(!audit_dummy_context()))
- __audit_log_capset(pid, new, old);
+ __audit_log_capset(new, old);
}

static inline void audit_mmap_fd(int fd, int flags)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 74a0748..60a966d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2316,18 +2316,16 @@ int __audit_log_bprm_fcaps(struct linux_binprm *bprm,

/**
* __audit_log_capset - store information about the arguments to the capset syscall
- * @pid: target pid of the capset call
* @new: the new credentials
* @old: the old (current) credentials
*
* Record the aguments userspace sent to sys_capset for later printing by the
* audit system if applicable
*/
-void __audit_log_capset(pid_t pid,
- const struct cred *new, const struct cred *old)
+void __audit_log_capset(const struct cred *new, const struct cred *old)
{
struct audit_context *context = current->audit_context;
- context->capset.pid = pid;
+ context->capset.pid = task_pid_nr_init_ns(current);
context->capset.cap.effective = new->cap_effective;
context->capset.cap.inheritable = new->cap_effective;
context->capset.cap.permitted = new->cap_permitted;
diff --git a/kernel/capability.c b/kernel/capability.c
index f6c2ce5..7f60311 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -277,7 +277,7 @@ SYSCALL_DEFINE2(capset, cap_user_header_t, header, const cap_user_data_t, data)
if (ret < 0)
goto error;

- audit_log_capset(pid, new, current_cred());
+ audit_log_capset(new, current_cred());

return commit_creds(new);
--
1.7.1
Richard Guy Briggs
2013-08-20 21:32:02 UTC
Permalink
task->tgid is an error prone construct and results in duplicate maintenance.
Start it's demise by modifying task_tgid_nr to not use it.

Cc: "Eric W. Biederman" <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/sched.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index fb09b93..8e69807 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1495,7 +1495,7 @@ static inline pid_t task_pid_vnr(struct task_struct *tsk)

static inline pid_t task_tgid_nr(struct task_struct *tsk)
{
- return tsk->tgid;
+ return pid_nr(task_tgid(tsk));
}

pid_t task_tgid_nr_ns(struct task_struct *tsk, struct pid_namespace *ns);
--
1.7.1
Richard Guy Briggs
2013-08-20 21:32:03 UTC
Permalink
This stops these four task helper functions from using the deprecated and
error-prone task->pid and task->tgid.

(informed by ebiederman's ea5a4d01)
Cc: "Eric W. Biederman" <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/sched.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8e69807..46e739d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1579,7 +1579,7 @@ static inline int pid_alive(struct task_struct *p)
*/
static inline int is_global_init(struct task_struct *tsk)
{
- return tsk->pid == 1;
+ return task_pid_nr(tsk) == 1;
}

extern struct pid *cad_pid;
@@ -1930,7 +1930,7 @@ extern struct task_struct *idle_task(int cpu);
*/
static inline bool is_idle_task(const struct task_struct *p)
{
- return p->pid == 0;
+ return task_pid(p) == &init_struct_pid;
}
extern struct task_struct *curr_task(int cpu);
extern void set_curr_task(int cpu, struct task_struct *p);
@@ -2203,13 +2203,13 @@ static inline bool thread_group_leader(struct task_struct *p)
*/
static inline int has_group_leader_pid(struct task_struct *p)
{
- return p->pid == p->tgid;
+ return task_pid(p) == task_tgid(p);
}

static inline
int same_thread_group(struct task_struct *p1, struct task_struct *p2)
{
- return p1->tgid == p2->tgid;
+ return task_tgid(p1) == task_tgid(p2);
}

static inline struct task_struct *next_thread(const struct task_struct *p)
--
1.7.1
Oleg Nesterov
2013-08-22 19:08:48 UTC
Permalink
Post by Richard Guy Briggs
static inline int is_global_init(struct task_struct *tsk)
{
- return tsk->pid == 1;
+ return task_pid_nr(tsk) == 1;
}
Probably it would be better to simply kill it. Almost every usage is
wrong.
Post by Richard Guy Briggs
static inline bool is_idle_task(const struct task_struct *p)
{
- return p->pid == 0;
+ return task_pid(p) == &init_struct_pid;
}
hmm. there should be a simpler check for this...
Post by Richard Guy Briggs
static inline int has_group_leader_pid(struct task_struct *p)
{
- return p->pid == p->tgid;
+ return task_pid(p) == task_tgid(p);
}
static inline
int same_thread_group(struct task_struct *p1, struct task_struct *p2)
{
- return p1->tgid == p2->tgid;
+ return task_tgid(p1) == task_tgid(p2);
This is suboptinal. See the attached
include-linux-schedh-dont-use-task-pid-tgid-in-same_thread_group-has_group_leader_pid.patch
from -mm below.

Oleg.


--- a/include/linux/sched.h~include-linux-schedh-dont-use-task-pid-tgid-in-same_thread_group-has_group_leader_pid
+++ a/include/linux/sched.h
@@ -2179,15 +2179,15 @@ static inline bool thread_group_leader(s
* all we care about is that we have a task with the appropriate
* pid, we don't actually care if we have the right task.
*/
-static inline int has_group_leader_pid(struct task_struct *p)
+static inline bool has_group_leader_pid(struct task_struct *p)
{
- return p->pid == p->tgid;
+ return task_pid(p) == p->signal->leader_pid;
}

static inline
-int same_thread_group(struct task_struct *p1, struct task_struct *p2)
+bool same_thread_group(struct task_struct *p1, struct task_struct *p2)
{
- return p1->tgid == p2->tgid;
+ return p1->signal == p2->signal;
}

static inline struct task_struct *next_thread(const struct task_struct *p)

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Peter Zijlstra
2013-08-22 20:05:55 UTC
Permalink
Post by Richard Guy Briggs
This stops these four task helper functions from using the deprecated and
error-prone task->pid and task->tgid.
(informed by ebiederman's ea5a4d01)
---
include/linux/sched.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8e69807..46e739d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1579,7 +1579,7 @@ static inline int pid_alive(struct task_struct *p)
*/
static inline int is_global_init(struct task_struct *tsk)
{
- return tsk->pid == 1;
+ return task_pid_nr(tsk) == 1;
}
extern struct pid *cad_pid;
@@ -1930,7 +1930,7 @@ extern struct task_struct *idle_task(int cpu);
*/
static inline bool is_idle_task(const struct task_struct *p)
{
- return p->pid == 0;
+ return task_pid(p) == &init_struct_pid;
}
extern struct task_struct *curr_task(int cpu);
extern void set_curr_task(int cpu, struct task_struct *p);
Why would you ever want to do this? It just makes these tests more
expensive for no gain what so ff'ing ever.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Richard Guy Briggs
2013-08-20 21:31:59 UTC
Permalink
- PID will be reported in the relevant querying PID namespace.

- Refuse to change the current audit_pid if the new value does not
point to an existing process.

- Refuse to set the current audit_pid if the new value is not its own PID
(unless it is being unset).

- Convert audit_pid into the initial pid namespace for reports

(informed by ebiederman's 5bf431da)
Cc: "Eric W. Biederman" <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/audit.c | 25 +++++++++++++++++++------
kernel/audit.h | 4 ++--
kernel/auditsc.c | 6 +++---
3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 75b0989..fa1d5f5 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -93,7 +93,7 @@ static int audit_failure = AUDIT_FAIL_PRINTK;
* contains the pid of the auditd process and audit_nlk_portid contains
* the portid to use to send netlink messages to that process.
*/
-int audit_pid;
+struct pid *audit_pid;
static __u32 audit_nlk_portid;

/* If audit_rate_limit is non-zero, limit the rate of sending audit records
@@ -388,9 +388,11 @@ static void kauditd_send_skb(struct sk_buff *skb)
err = netlink_unicast(audit_sock, skb, audit_nlk_portid, 0);
if (err < 0) {
BUG_ON(err != -ECONNREFUSED); /* Shouldn't happen */
- printk(KERN_ERR "audit: *NO* daemon at audit_pid=%d\n", audit_pid);
+ pr_err("audit: *NO* daemon at audit_pid=%d\n"
+ , pid_nr_init_ns(audit_pid));
audit_log_lost("auditd disappeared\n");
- audit_pid = 0;
+ put_pid(audit_pid);
+ audit_pid = NULL;
/* we might get lucky and get this in the next auditd */
audit_hold_skb(skb);
} else
@@ -661,7 +663,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
case AUDIT_GET:
status_set.enabled = audit_enabled;
status_set.failure = audit_failure;
- status_set.pid = audit_pid;
+ status_set.pid = pid_vnr(audit_pid);
status_set.rate_limit = audit_rate_limit;
status_set.backlog_limit = audit_backlog_limit;
status_set.lost = atomic_read(&audit_lost);
@@ -684,10 +686,21 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return err;
}
if (status_get->mask & AUDIT_STATUS_PID) {
- int new_pid = status_get->pid;
+ struct pid *new_pid = find_get_pid(status_get->pid);
+ if (status_get->pid && !new_pid)
+ return -ESRCH;
+
+ /* check that non-zero pid it is trying to set is its
+ * own*/
+ if (status_get->pid &&
+ (status_get->pid != task_pid_vnr(current)))
+ return -EPERM;

if (audit_enabled != AUDIT_OFF)
- audit_log_config_change("audit_pid", new_pid, audit_pid, 1);
+ audit_log_config_change("audit_pid",
+ pid_nr_init_ns(new_pid),
+ pid_nr_init_ns(audit_pid),
+ 1);
audit_pid = new_pid;
audit_nlk_portid = NETLINK_CB(skb).portid;
}
diff --git a/kernel/audit.h b/kernel/audit.h
index 36edcf5..3932a3b 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, struct path *path,
int record_num, int *call_panic);

-extern int audit_pid;
+extern struct pid *audit_pid;

#define AUDIT_INODE_BUCKETS 32
extern struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];
@@ -310,7 +310,7 @@ extern u32 audit_sig_sid;
extern int __audit_signal_info(int sig, struct task_struct *t);
static inline int audit_signal_info(int sig, struct task_struct *t)
{
- if (unlikely((audit_pid && t->tgid == audit_pid) ||
+ if (unlikely((audit_pid && task_tgid(t) == audit_pid) ||
(audit_signals && !audit_dummy_context())))
return __audit_signal_info(sig, t);
return 0;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 60a966d..2dcf67d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -739,7 +739,7 @@ static enum audit_state audit_filter_syscall(struct task_struct *tsk,
struct audit_entry *e;
enum audit_state state;

- if (audit_pid && tsk->tgid == audit_pid)
+ if (audit_pid && task_tgid(tsk) == audit_pid)
return AUDIT_DISABLED;

rcu_read_lock();
@@ -800,7 +800,7 @@ void audit_filter_inodes(struct task_struct *tsk, struct audit_context *ctx)
{
struct audit_names *n;

- if (audit_pid && tsk->tgid == audit_pid)
+ if (audit_pid && task_tgid(tsk) == audit_pid)
return;

rcu_read_lock();
@@ -2220,7 +2220,7 @@ int __audit_signal_info(int sig, struct task_struct *t)
struct audit_context *ctx = tsk->audit_context;
kuid_t uid = current_uid(), t_uid = task_uid(t);

- if (audit_pid && t->tgid == audit_pid) {
+ if (audit_pid && task_tgid(t) == audit_pid) {
if (sig == SIGTERM || sig == SIGHUP || sig == SIGUSR1 || sig == SIGUSR2) {
audit_sig_pid = tsk->pid;
if (uid_valid(tsk->loginuid))
--
1.7.1
Richard Guy Briggs
2013-08-20 21:31:57 UTC
Permalink
Added the functions
task_pid_nr_init_ns()
task_tgid_nr_init_ns()
to avoid the use of the error-prone duplicity of task->pid and task->tgid,
avoid changing the existing usage of task_pid_nr() and task_tgid_nr() while it
gets converted away from task->pid and task->tgid, and provide a clear
abstraction of the frequently used init_pid_ns in task_pid_nr_ns() and
task_tgid_nr_ns().
Also added pid_nr_init_ns() to explicitly use init_pid_ns.

(informed by ebiederman's 3a2e8c59)
Cc: Eric W. Biederman <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
include/linux/pid.h | 6 ++++++
include/linux/sched.h | 10 ++++++++++
2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/linux/pid.h b/include/linux/pid.h
index 23705a5..051e134 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -172,6 +172,12 @@ static inline pid_t pid_nr(struct pid *pid)
pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns);
pid_t pid_vnr(struct pid *pid);

+static inline pid_t pid_nr_init_ns(struct pid *pid)
+{
+ return pid_nr_ns(pid, &init_pid_ns);
+}
+
+
#define do_each_pid_task(pid, type, task) \
do { \
if ((pid) != NULL) \
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ec04090..9651d95 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1482,6 +1482,11 @@ static inline pid_t task_pid_nr_ns(struct task_struct *tsk,
return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns);
}

+static inline pid_t task_pid_nr_init_ns(struct task_struct *tsk)
+{
+ return task_pid_nr_ns(tsk, &init_pid_ns);
+}
+
static inline pid_t task_pid_vnr(struct task_struct *tsk)
{
return __task_pid_nr_ns(tsk, PIDTYPE_PID, NULL);
@@ -1495,6 +1500,11 @@ static inline pid_t task_tgid_nr(struct task_struct *tsk)

pid_t task_tgid_nr_ns(struct task_struct *tsk, struct pid_namespace *ns);

+static inline pid_t task_tgid_nr_init_ns(struct task_struct *tsk)
+{
+ return task_tgid_nr_ns(tsk, &init_pid_ns);
+}
+
static inline pid_t task_tgid_vnr(struct task_struct *tsk)
{
return pid_vnr(task_tgid(tsk));
--
1.7.1
Richard Guy Briggs
2013-08-20 21:31:56 UTC
Permalink
sys_getppid() returns the parent pid of the current process in its own pid
namespace. Since audit filters are based in the init pid namespace, a process
could avoid a filter or trigger an unintended one by being in an alternate pid
namespace or log meaningless information.

Switch to task_ppid_nr_init_ns() for PPIDs to anchor all audit filters in the
init_pid_ns.

(informed by ebiederman's 6c621b7e)
Cc: ***@vger.kernel.org
Cc: Eric W. Biederman <***@xmission.com>
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
kernel/audit.c | 4 ++--
kernel/auditsc.c | 2 +-
security/apparmor/audit.c | 5 +----
3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 2476334..75b0989 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1588,10 +1588,10 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
spin_unlock_irq(&tsk->sighand->siglock);

audit_log_format(ab,
- " ppid=%ld pid=%d auid=%u uid=%u gid=%u"
+ " ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
" egid=%u sgid=%u fsgid=%u ses=%u tty=%s",
- sys_getppid(),
+ task_ppid_nr_init_ns(tsk),
tsk->pid,
from_kuid(&init_user_ns, audit_get_loginuid(tsk)),
from_kuid(&init_user_ns, cred->uid),
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8c644c5..74a0748 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -466,7 +466,7 @@ static int audit_filter_rules(struct task_struct *tsk,
case AUDIT_PPID:
if (ctx) {
if (!ctx->ppid)
- ctx->ppid = sys_getppid();
+ ctx->ppid = task_ppid_nr_init_ns(current);
result = audit_comparator(ctx->ppid, f->op, f->val);
}
break;
diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 031d2d9..497b306 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -132,10 +132,7 @@ static void audit_pre(struct audit_buffer *ab, void *ca)

if (sa->aad->profile) {
struct aa_profile *profile = sa->aad->profile;
- pid_t pid;
- rcu_read_lock();
- pid = rcu_dereference(tsk->real_parent)->pid;
- rcu_read_unlock();
+ pid_t pid = task_ppid_nr_init_ns(tsk);
audit_log_format(ab, " parent=%d", pid);
if (profile->ns != root_ns) {
audit_log_format(ab, " namespace=");
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Continue reading on narkive:
Loading...