Re: [PATCH] audit: Use struct net not pid_t to remember the network namespce to reply in

On 14/02/28, Eric W. Biederman wrote: > > While reading through 3.14-rc1 I found a pretty siginficant mishandling > of network namespaces in the recent audit changes. > > In struct audit_netlink_list and audit_reply add a reference to the > network namespace of the caller and remove the userspace pid of the > caller. This cleanly remembers the callers network namespace, and > removes a huge class of races and nasty failure modes that can occur > when attempting to relook up the callers network namespace from a pid_t > (including the caller's network namespace changing, pid wraparound, and > the pid simply not being present). > > Signed-off-by: "Eric W. Biederman" <ebiederm(a)xmission.com&gt; Signed-off-by: Richard Guy Briggs <rgb(a)tricolour.ca&gt;

> --- > kernel/audit.c | 10 ++++++---- > kernel/audit.h | 2 +- > kernel/auditfilter.c | 3 ++- > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 34c5a2310fbf..1e5756f16f6f 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -182,7 +182,7 @@ struct audit_buffer { > > struct audit_reply { > __u32 portid; > - pid_t pid; > + struct net *net; > struct sk_buff *skb; > }; > > @@ -500,7 +500,7 @@ int audit_send_list(void *_dest) > { > struct audit_netlink_list *dest = _dest; > struct sk_buff *skb; > - struct net *net = get_net_ns_by_pid(dest->pid); > + struct net *net = dest->net; > struct audit_net *aunet = net_generic(net, audit_net_id); > > /* wait for parent to finish and send an ACK */ > @@ -510,6 +510,7 @@ int audit_send_list(void *_dest) > while ((skb = __skb_dequeue(&dest->q)) != NULL) > netlink_unicast(aunet->nlsk, skb, dest->portid, 0); > > + put_net(net); > kfree(dest); > > return 0; > @@ -543,7 +544,7 @@ out_kfree_skb: > static int audit_send_reply_thread(void *arg) > { > struct audit_reply *reply = (struct audit_reply *)arg; > - struct net *net = get_net_ns_by_pid(reply->pid); > + struct net *net = reply->net; > struct audit_net *aunet = net_generic(net, audit_net_id); > > mutex_lock(&audit_cmd_mutex); > @@ -552,6 +553,7 @@ 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(aunet->nlsk , reply->skb, reply->portid, 0); > + put_net(net); > kfree(reply); > return 0; > } > @@ -583,8 +585,8 @@ static void audit_send_reply(__u32 portid, int seq, int type, int done, > if (!skb) > goto out; > > + reply->net = get_net(current->nsproxy->net_ns); > reply->portid = portid; > - reply->pid = task_pid_vnr(current); > reply->skb = skb; > > tsk = kthread_run(audit_send_reply_thread, reply, "audit_send_reply"); > diff --git a/kernel/audit.h b/kernel/audit.h > index 57cc64d67718..8df132214606 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -247,7 +247,7 @@ extern void audit_panic(const char *message); > > struct audit_netlink_list { > __u32 portid; > - pid_t pid; > + struct net *net; > struct sk_buff_head q; > }; > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 14a78cca384e..a5e3d73d73e4 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -29,6 +29,7 @@ > #include <linux/sched.h> > #include <linux/slab.h> > #include <linux/security.h> > +#include <net/net_namespace.h> > #include "audit.h" > > /* > @@ -1083,8 +1084,8 @@ int audit_list_rules_send(__u32 portid, int seq) > dest = kmalloc(sizeof(struct audit_netlink_list), GFP_KERNEL); > if (!dest) > return -ENOMEM; > + dest->net = get_net(current->nsproxy->net_ns); > dest->portid = portid; > - dest->pid = task_pid_vnr(current); > skb_queue_head_init(&dest->q); > > mutex_lock(&audit_filter_mutex); > -- > 1.7.5.4 > - RGB -- Richard Guy Briggs <rbriggs(a)redhat.com&gt; Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545