netdev - Re: [PATCH] nf_conntrack: avoid kernel pointer value leak in slab name

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: <1463251566.18194.31.camel@edumazet-glaptop3.roam.corp.google.com>
Date:	Sat, 14 May 2016 11:46:06 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Miller <davem@...emloft.net>, Willy Tarreau <w@....eu>,
	Network Development List <netdev@...r.kernel.org>
Subject: Re: [PATCH] nf_conntrack: avoid kernel pointer value leak in slab
 name

On Sat, 2016-05-14 at 11:24 -0700, Linus Torvalds wrote:
> From: Linus Torvalds <torvalds@...ux-foundation.org>
> Date: Sat, 14 May 2016 11:11:44 -0700
> Subject: [PATCH] nf_conntrack: avoid kernel pointer value leak in slab name
> 
> The slab name ends up being visible in the directory structure under
> /sys, and even if you don't have access rights to the file you can see
> the filenames.
> 
> Just use a 64-bit counter instead of the pointer to the 'net' structure
> to generate a unique name.
> 
> This code will go away in 4.7 when the conntrack code moves to a single
> kmemcache, but this is the backportable simple solution to avoiding
> leaking kernel pointers to user space.
> 
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Acked-by: Eric Dumazet <eric.dumazet@...il.com>
> Cc: stable@...r.kernel.org
> ---
> 
> This would seem to be the minimal patch.
> 
> Eric - I marked you as "acking" this patch from the discussion. It's not 
> actually any of the exact patches that were flying around, but close 
> enough..
> 
> It's been "tested" by booting and looking at the end result. Seems to 
> work, and it's not exactly complicated.
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 895d11dced3c..e27fd17c6743 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1778,6 +1778,7 @@ void nf_conntrack_init_end(void)
>  
>  int nf_conntrack_init_net(struct net *net)
>  {
> +	static atomic64_t unique_id;
>  	int ret = -ENOMEM;
>  	int cpu;
>  
> @@ -1800,7 +1801,8 @@ int nf_conntrack_init_net(struct net *net)
>  	if (!net->ct.stat)
>  		goto err_pcpu_lists;
>  
> -	net->ct.slabname = kasprintf(GFP_KERNEL, "nf_conntrack_%p", net);
> +	net->ct.slabname = kasprintf(GFP_KERNEL, "nf_conntrack_%llu",
> +				(u64)atomic64_inc_return(&unique_id));
>  	if (!net->ct.slabname)
>  		goto err_slabname;
>  

SGTM, thanks Linus.

Fixes: 5b3501faa874 ("netfilter: nf_conntrack: per netns nf_conntrack_cachep")