netdev - Re: [PATCH] netns: fix net_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F212E7E.2040801@parallels.com>
Date:	Thu, 26 Jan 2012 14:44:14 +0400
From:	Pavel Emelyanov <xemul@...allels.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Sjur Brændeland 
	<sjur.brandeland@...ricsson.com>,
	"levinsasha928@...il.com" <levinsasha928@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"davej@...hat.com" <davej@...hat.com>,
	"sjurbren@...il.com" <sjurbren@...il.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH] netns: fix net_alloc_generic()

> I believe the problem is in net_namespace infrastructure, not in CAIF.
> 
> Could you test following patch instead ?
> 
> [PATCH] netns: fix net_alloc_generic()
> 
> When a new net namespace is created, we should attach to it a "struct
> net_generic" with enough slots (even empty), or we can hit the following
> BUG_ON() :
> 
> [  200.752016] kernel BUG at include/net/netns/generic.h:40!
> ...
> [  200.752016]  [<ffffffff825c3cea>] ? get_cfcnfg+0x3a/0x180
> [  200.752016]  [<ffffffff821cf0b0>] ? lockdep_rtnl_is_held+0x10/0x20
> [  200.752016]  [<ffffffff825c41be>] caif_device_notify+0x2e/0x530
> [  200.752016]  [<ffffffff810d61b7>] notifier_call_chain+0x67/0x110
> [  200.752016]  [<ffffffff810d67c1>] raw_notifier_call_chain+0x11/0x20
> [  200.752016]  [<ffffffff821bae82>] call_netdevice_notifiers+0x32/0x60
> [  200.752016]  [<ffffffff821c2b26>] register_netdevice+0x196/0x300
> [  200.752016]  [<ffffffff821c2ca9>] register_netdev+0x19/0x30
> [  200.752016]  [<ffffffff81c1c67a>] loopback_net_init+0x4a/0xa0
> [  200.752016]  [<ffffffff821b5e62>] ops_init+0x42/0x180
> [  200.752016]  [<ffffffff821b600b>] setup_net+0x6b/0x100
> [  200.752016]  [<ffffffff821b6466>] copy_net_ns+0x86/0x110
> [  200.752016]  [<ffffffff810d5789>] create_new_namespaces+0xd9/0x190
> 
> net_alloc_generic() should take into account the maximum index into the
> ptr array, as a subsystem might use net_generic() anytime.

I'm not sure I understand it correctly, but subsystem can only use the
net_generic() only (!) after the net_assign_generic() is performed.

> This also reduces number of reallocations in net_assign_generic()
> 
> Reported-by: Sasha Levin <levinsasha928@...il.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
> Cc: Sjur Brændeland <sjur.brandeland@...ricsson.com>
> Cc: Eric W. Biederman <ebiederm@...ssion.com>
> Cc: Pavel Emelyanov <xemul@...nvz.org>
> ---
>  net/core/net_namespace.c |   31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index aefcd7a..0e950fd 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -30,6 +30,20 @@ EXPORT_SYMBOL(init_net);
>  
>  #define INITIAL_NET_GEN_PTRS	13 /* +1 for len +2 for rcu_head */
>  
> +static unsigned int max_gen_ptrs = INITIAL_NET_GEN_PTRS;
> +
> +static struct net_generic *net_alloc_generic(void)
> +{
> +	struct net_generic *ng;
> +	size_t generic_size = offsetof(struct net_generic, ptr[max_gen_ptrs]);
> +
> +	ng = kzalloc(generic_size, GFP_KERNEL);
> +	if (ng)
> +		ng->len = max_gen_ptrs;
> +
> +	return ng;
> +}
> +
>  static int net_assign_generic(struct net *net, int id, void *data)
>  {
>  	struct net_generic *ng, *old_ng;
> @@ -43,8 +57,7 @@ static int net_assign_generic(struct net *net, int id, void *data)
>  	if (old_ng->len >= id)
>  		goto assign;
>  
> -	ng = kzalloc(sizeof(struct net_generic) +
> -			id * sizeof(void *), GFP_KERNEL);
> +	ng = net_alloc_generic();
>  	if (ng == NULL)
>  		return -ENOMEM;
>  
> @@ -59,7 +72,6 @@ static int net_assign_generic(struct net *net, int id, void *data)
>  	 * the old copy for kfree after a grace period.
>  	 */
>  
> -	ng->len = id;
>  	memcpy(&ng->ptr, &old_ng->ptr, old_ng->len * sizeof(void*));
>  
>  	rcu_assign_pointer(net->gen, ng);
> @@ -161,18 +173,6 @@ out_undo:
>  	goto out;
>  }
>  
> -static struct net_generic *net_alloc_generic(void)
> -{
> -	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;
> -}
>  
>  #ifdef CONFIG_NET_NS
>  static struct kmem_cache *net_cachep;
> @@ -483,6 +483,7 @@ again:
>  			}
>  			return error;
>  		}
> +		max_gen_ptrs = max_t(unsigned int, max_gen_ptrs, *ops->id);
>  	}
>  	error = __register_pernet_operations(list, ops);
>  	if (error) {
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html