diff mbox

net/udp: slab-out-of-bounds Read in udp_recvmsg

Message ID 1489594233.28631.152.camel@edumazet-glaptop3.roam.corp.google.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet March 15, 2017, 4:10 p.m. UTC 
On Wed, 2017-03-15 at 09:01 -0700, Eric Dumazet wrote:
> On Wed, 2017-03-15 at 16:41 +0100, Dmitry Vyukov wrote:
> > On Wed, Mar 15, 2017 at 4:25 PM, 쪼르 <zzoru007@gmail.com> wrote:
> > > It seems that attacker can leak kernel memory(slab) by this vulnerability.
> > > I make a PoC code, and it works well on
> > > ae50dfd61665086e617cc9e554a1285d52765670.
> > > but, I found that PoC wasn't work on Ubuntu16.04.02 4.4.0-64-generic
> > > #85-Ubuntu SMP.
> > 
> > 
> > Do you know why it is not working on Ubuntu16.04.02?
> > Is it because the source bug is not present there? Or maybe you need a
> > slightly different poc for that version?
> > 
> 
> Seems to be a side effect of a recent commit
> 
> ( 1c885808e45601b2b6f68b30ac1d999e10b6f606 )


Can you try this fix ?

Comments

David Miller March 15, 2017, 10:08 p.m. UTC | #1 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 15 Mar 2017 09:10:33 -0700

> @@ -692,12 +692,17 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
>  	    ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2))
>  		empty = 0;
>  	if (!empty) {
> +		unsigned int hlen = skb_headlen(skb);
> +
>  		put_cmsg(msg, SOL_SOCKET,
>  			 SCM_TIMESTAMPING, sizeof(tss), &tss);
>  
> -		if (skb->len && (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS))
> +		if (hlen &&
> +		    (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
> +		    sk->sk_protocol == IPPROTO_TCP &&
> +		    sk->sk_type == SOCK_STREAM)
>  			put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_OPT_STATS,
> -				 skb->len, skb->data);
> +				 hlen, skb->data);

Hmmm, what is the true intention of SOF_TIMESTAMPING_OPT_STATS then?  The
existing code seems to want to dump the entire SKB into the cmsg, and if
that's the case then the fix is to linearlize the skb before the put_cmsg()
or have a way to put a non-linear SKB into a cmsg.
Eric Dumazet March 15, 2017, 10:45 p.m. UTC | #2 
On Wed, 2017-03-15 at 15:08 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 15 Mar 2017 09:10:33 -0700
> 
> > @@ -692,12 +692,17 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
> >  	    ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2))
> >  		empty = 0;
> >  	if (!empty) {
> > +		unsigned int hlen = skb_headlen(skb);
> > +
> >  		put_cmsg(msg, SOL_SOCKET,
> >  			 SCM_TIMESTAMPING, sizeof(tss), &tss);
> >  
> > -		if (skb->len && (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS))
> > +		if (hlen &&
> > +		    (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
> > +		    sk->sk_protocol == IPPROTO_TCP &&
> > +		    sk->sk_type == SOCK_STREAM)
> >  			put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_OPT_STATS,
> > -				 skb->len, skb->data);
> > +				 hlen, skb->data);
> 
> Hmmm, what is the true intention of SOF_TIMESTAMPING_OPT_STATS then?  The
> existing code seems to want to dump the entire SKB into the cmsg, and if
> that's the case then the fix is to linearlize the skb before the put_cmsg()
> or have a way to put a non-linear SKB into a cmsg.

I simply matched the conditions in __skb_tstamp_tx() which builds the
skb :

+       if (tsonly) {
+#ifdef CONFIG_INET
+               if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
+                   sk->sk_protocol == IPPROTO_TCP &&
+                   sk->sk_type == SOCK_STREAM)
+                       skb = tcp_get_timestamping_opt_stats(sk);
+               else
+#endif
+                       skb = alloc_skb(0, GFP_ATOMIC);
+       } else {


And note that I should have also used the #ifdef


A proper fix would be to find a bit in skb->cb[] to avoid duplicating
the test...
diff mbox

Patch

diff --git a/net/socket.c b/net/socket.c
index e034fe4164beec7731c68ba2bc6920627741561b..9b9a8eca81efa4d310be4376eb07c12614f7b562 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -692,12 +692,17 @@  void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	    ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2))
 		empty = 0;
 	if (!empty) {
+		unsigned int hlen = skb_headlen(skb);
+
 		put_cmsg(msg, SOL_SOCKET,
 			 SCM_TIMESTAMPING, sizeof(tss), &tss);
 
-		if (skb->len && (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS))
+		if (hlen &&
+		    (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
+		    sk->sk_protocol == IPPROTO_TCP &&
+		    sk->sk_type == SOCK_STREAM)
 			put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPING_OPT_STATS,
-				 skb->len, skb->data);
+				 hlen, skb->data);
 	}
 }
 EXPORT_SYMBOL_GPL(__sock_recv_timestamp);