netdev - Re: [RFC PATCH] net: cgroup: null ptr dereference in netprio cgroup during init

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: <20120718124539.GC25563@hmsreliant.think-freely.org>
Date:	Wed, 18 Jul 2012 08:45:39 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	John Fastabend <john.r.fastabend@...el.com>
Cc:	davem@...emloft.net, gaofeng@...fujitsu.com,
	mark.d.rustad@...el.com, netdev@...r.kernel.org,
	eric.dumazet@...il.com
Subject: Re: [RFC PATCH] net: cgroup: null ptr dereference in netprio cgroup
 during init

On Tue, Jul 17, 2012 at 05:33:16PM -0700, John Fastabend wrote:
> When the netprio cgroup is built in the kernel cgroup_init will call
> cgrp_create which eventually calls update_netdev_tables. This is
> being called before do_initcalls() so a null ptr dereference occurs
> on init_net.
> 
> This patch adds a check on init_net.count to verify the structure
> has been initialized. The failure was introduced here,
> 
> commit ef209f15980360f6945873df3cd710c5f62f2a3e
> Author: Gao feng <gaofeng@...fujitsu.com>
> Date:   Wed Jul 11 21:50:15 2012 +0000
> 
>     net: cgroup: fix access the unallocated memory in netprio cgroup
> 
> Tested with ping with netprio_cgroup as a module and built in.
> 
> Marked RFC for now I think DaveM might have a reason why this needs
> some improvement.
> 
> Reported-by: Mark Rustad <mark.d.rustad@...el.com>
> Cc: Neil Horman <nhorman@...driver.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Gao feng <gaofeng@...fujitsu.com>
> Signed-off-by: John Fastabend <john.r.fastabend@...el.com>
> ---
> 
>  net/core/netprio_cgroup.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
> index b2e9caa..e9fd7fd 100644
> --- a/net/core/netprio_cgroup.c
> +++ b/net/core/netprio_cgroup.c
> @@ -116,6 +116,9 @@ static int update_netdev_tables(void)
>  	u32 max_len;
>  	struct netprio_map *map;
>  
> +	if (!atomic_read(&init_net.count))
> +		return ret;
> +
>  	rtnl_lock();
>  	max_len = atomic_read(&max_prioidx) + 1;
>  	for_each_netdev(&init_net, dev) {
> 
> 

John, do you have a stack trace of this.  I'm having a hard time seeing how we
get into this path prior to the network stack being initalized.

It also brings up another point.  If this is happening, and we're creating the
root cgroup from start_kernel, Then we're actually initalizing some cgroups
twice, because a few cgroups register themselves via cgroup_load_subsys in
module_init specified routines.  So if you're building netprio_cgroup or
net_cls_cgroup as part of the monolithic kernel, you'll get cgroup_create called
prior to your module_init() call.  Thats not good.

In fact, the cgroup_subsys struct has an early_init flag that cgroup_init
appears to use to skip the initialization of subsystems that don't need to be
initialized that early in boot (assuming thats the path we're going down to get
to this oops).  

If you can post the call stack, I'd appreciate it, I'd like to dig a bit deeper
into this.
Neil

--
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