From 486a87f1e5624096bd1c09e9e716239597d48dca Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Sun, 22 Feb 2009 00:07:53 -0800 Subject: netns: fix double free at netns creation This patch fix a double free when a network namespace fails. The previous code does a kfree of the net_generic structure when one of the init subsystem initialization fails. The 'setup_net' function does kfree(ng) and returns an error. The caller, 'copy_net_ns', call net_free on error, and this one calls kfree(net->gen), making this pointer freed twice. This patch make the code symetric, the net_alloc does the net_generic allocation and the net_free frees the net_generic. Signed-off-by: Daniel Lezcano Signed-off-by: David S. Miller --- net/core/net_namespace.c | 86 +++++++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 31 deletions(-) (limited to 'net/core/net_namespace.c') diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 55151faaf90..b0767abf23e 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -32,24 +32,14 @@ static __net_init int setup_net(struct net *net) { /* Must be called with net_mutex held */ struct pernet_operations *ops; - int error; - struct net_generic *ng; + int error = 0; atomic_set(&net->count, 1); + #ifdef NETNS_REFCNT_DEBUG atomic_set(&net->use_count, 0); #endif - error = -ENOMEM; - ng = kzalloc(sizeof(struct net_generic) + - INITIAL_NET_GEN_PTRS * sizeof(void *), GFP_KERNEL); - if (ng == NULL) - goto out; - - ng->len = INITIAL_NET_GEN_PTRS; - rcu_assign_pointer(net->gen, ng); - - error = 0; list_for_each_entry(ops, &pernet_list, list) { if (ops->init) { error = ops->init(net); @@ -70,7 +60,6 @@ out_undo: } rcu_barrier(); - kfree(ng); goto out; } @@ -78,16 +67,43 @@ out_undo: static struct kmem_cache *net_cachep; static struct workqueue_struct *netns_wq; -static struct net *net_alloc(void) +static struct net_generic *net_alloc_generic(void) { - return kmem_cache_zalloc(net_cachep, GFP_KERNEL); + struct net_generic *ng; + size_t generic_size = sizeof(struct net_generic) + + INITIAL_NET_GEN_PTRS * sizeof(void *); + + ng = kzalloc(generic_size, GFP_KERNEL); + if (ng) + ng->len = INITIAL_NET_GEN_PTRS; + + return ng; } -static void net_free(struct net *net) +static struct net *net_alloc(void) { + struct net *net = NULL; + struct net_generic *ng; + + ng = net_alloc_generic(); + if (!ng) + goto out; + + net = kmem_cache_zalloc(net_cachep, GFP_KERNEL); if (!net) - return; + goto out_free; + + rcu_assign_pointer(net->gen, ng); +out: + return net; + +out_free: + kfree(ng); + goto out; +} +static void net_free(struct net *net) +{ #ifdef NETNS_REFCNT_DEBUG if (unlikely(atomic_read(&net->use_count) != 0)) { printk(KERN_EMERG "network namespace not free! Usage: %d\n", @@ -112,27 +128,28 @@ struct net *copy_net_ns(unsigned long flags, struct net *old_net) err = -ENOMEM; new_net = net_alloc(); if (!new_net) - goto out; + goto out_err; mutex_lock(&net_mutex); err = setup_net(new_net); - if (err) - goto out_unlock; - - rtnl_lock(); - list_add_tail(&new_net->list, &net_namespace_list); - rtnl_unlock(); - - -out_unlock: + if (!err) { + rtnl_lock(); + list_add_tail(&new_net->list, &net_namespace_list); + rtnl_unlock(); + } mutex_unlock(&net_mutex); + + if (err) + goto out_free; out: put_net(old_net); - if (err) { - net_free(new_net); - new_net = ERR_PTR(err); - } return new_net; + +out_free: + net_free(new_net); +out_err: + new_net = ERR_PTR(err); + goto out; } static void cleanup_net(struct work_struct *work) @@ -188,6 +205,7 @@ struct net *copy_net_ns(unsigned long flags, struct net *old_net) static int __init net_ns_init(void) { + struct net_generic *ng; int err; printk(KERN_INFO "net_namespace: %zd bytes\n", sizeof(struct net)); @@ -202,6 +220,12 @@ static int __init net_ns_init(void) panic("Could not create netns workq"); #endif + ng = net_alloc_generic(); + if (!ng) + panic("Could not allocate generic netns"); + + rcu_assign_pointer(init_net.gen, ng); + mutex_lock(&net_mutex); err = setup_net(&init_net); -- cgit v1.2.3 From ebe47d47b7b7fed72dabcce4717da727b4e2367d Mon Sep 17 00:00:00 2001 From: Clemens Noss Date: Mon, 23 Feb 2009 15:37:35 -0800 Subject: netns: build fix for net_alloc_generic net_alloc_generic was defined in #ifdef CONFIG_NET_NS, but used unconditionally. Move net_alloc_generic out of #ifdef. Signed-off-by: Clemens Noss Signed-off-by: David S. Miller --- net/core/net_namespace.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/core/net_namespace.c') diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index b0767abf23e..2adb1a7d361 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -63,10 +63,6 @@ out_undo: goto out; } -#ifdef CONFIG_NET_NS -static struct kmem_cache *net_cachep; -static struct workqueue_struct *netns_wq; - static struct net_generic *net_alloc_generic(void) { struct net_generic *ng; @@ -80,6 +76,10 @@ static struct net_generic *net_alloc_generic(void) return ng; } +#ifdef CONFIG_NET_NS +static struct kmem_cache *net_cachep; +static struct workqueue_struct *netns_wq; + static struct net *net_alloc(void) { struct net *net = NULL; -- cgit v1.2.3 From 17edde520927070a6bf14a6a75027c0b843443e5 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 22 Feb 2009 00:11:09 -0800 Subject: netns: Remove net_alive It turns out that net_alive is unnecessary, and the original problem that led to it being added was simply that the icmp code thought it was a network device and wound up being unable to handle packets while there were still packets in the network namespace. Now that icmp and tcp have been fixed to properly register themselves this problem is no longer present and we have a stronger guarantee that packets will not arrive in a network namespace then that provided by net_alive in netif_receive_skb. So remove net_alive allowing packet reception run a little faster. Additionally document the strong reason why network namespace cleanup is safe so that if something happens again someone else will have a chance of figuring it out. Signed-off-by: Eric W. Biederman Signed-off-by: David S. Miller --- net/core/net_namespace.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'net/core/net_namespace.c') diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 2adb1a7d361..e3bebd36f05 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -157,9 +157,6 @@ static void cleanup_net(struct work_struct *work) struct pernet_operations *ops; struct net *net; - /* Be very certain incoming network packets will not find us */ - rcu_barrier(); - net = container_of(work, struct net, work); mutex_lock(&net_mutex); -- cgit v1.2.3