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

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] [day] [month] [year] [list]

Date:   Wed, 15 Mar 2017 15:45:55 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     dvyukov@...gle.com, ycheng@...gle.com, soheil@...gle.com,
        ncardwell@...gle.com, zzoru007@...il.com, kuznet@....inr.ac.ru,
        jmorris@...ei.org, yoshfuji@...ux-ipv6.org, kaber@...sh.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzkaller@...glegroups.com
Subject: Re: net/udp: slab-out-of-bounds Read in udp_recvmsg

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