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

[Eric Dumazet]

On Wed, 2017-03-15 at 15:08 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@xxxxxxxxx>
> 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...