linux-kernel - Re: NULL pointer dereference in selinux_ip_postroute

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]

Date:	Wed, 08 Aug 2012 15:50:47 -0400
From:	Paul Moore <paul@...l-moore.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	John Stultz <johnstul@...ibm.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	lkml <linux-kernel@...r.kernel.org>,
	James Morris <james.l.morris@...cle.com>,
	selinux@...ho.nsa.gov, Eric Dumazet <edumazet@...gle.com>,
	john.johansen@...onical.com
Subject: Re: NULL pointer dereference in selinux_ip_postroute_compat

On Wednesday, August 08, 2012 09:38:21 PM Eric Dumazet wrote:
> On Wed, 2012-08-08 at 15:26 -0400, Paul Moore wrote:
> > On Wednesday, August 08, 2012 12:14:42 PM John Stultz wrote:
> > > So I bisected this down and it seems to be the following commit:
> > > 
> > > commit be9f4a44e7d41cee50ddb5f038fc2391cbbb4046
> > > Author: Eric Dumazet <edumazet@...gle.com>
> > > Date:   Thu Jul 19 07:34:03 2012 +0000
> > > 
> > >      ipv4: tcp: remove per net tcp_sock
> > > 
> > > It doesn't revert totally cleanly, but after fixing up the rejections
> > > and booting with this patch removed on top of Linus' head the oops on
> > > shutdown goes away.
> > 
> > Thanks!
> > 
> > It looks the like there is a bug in ip_send_unicast_reply() which uses a
> > inet_sock/sock struct which does not have the LSM data properly
> > initialized.
> > 
> > I'll put together a patch shortly.
> 
> Something like this ?

Yep.  I was just trying to see if there was a way we could avoid having to 
make it conditional on CONFIG_SECURITY, but I think this is a better approach 
than the alternatives.

I'm also looking into making sure we get a sane LSM label on the per-cpu sock 
as security_sk_alloc() just allocates and initializes the LSM blob to a basic 
starting value (unlabeled_t in the case of SELinux) ... that is likely to be 
the tricky bit.

Regardless, I'm okay with us merging the patch below now to fix the panic and 
I'll supply a follow-up patch to fix the labeling once I figure out a solution 
that seems reasonable.  Does that work for you?  David?

Acked-by: Paul Moore <paul@...l-moore.com>

> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index ba39a52..027a331 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1524,6 +1524,10 @@ void ip_send_unicast_reply(struct net *net, struct
> sk_buff *skb, __be32 daddr, sk->sk_priority = skb->priority;
>  	sk->sk_protocol = ip_hdr(skb)->protocol;
>  	sk->sk_bound_dev_if = arg->bound_dev_if;
> +#ifdef CONFIG_SECURITY
> +	if (!sk->sk_security && security_sk_alloc(sk, PF_INET, GFP_ATOMIC))
> +			goto out;
> +#endif
>  	sock_net_set(sk, net);
>  	__skb_queue_head_init(&sk->sk_write_queue);
>  	sk->sk_sndbuf = sysctl_wmem_default;
> @@ -1539,7 +1543,7 @@ void ip_send_unicast_reply(struct net *net, struct
> sk_buff *skb, __be32 daddr, skb_set_queue_mapping(nskb,
> skb_get_queue_mapping(skb));
>  		ip_push_pending_frames(sk, &fl4);
>  	}
> -
> +out:
>  	put_cpu_var(unicast_sock);
> 
>  	ip_rt_put(rt);

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/