| Message ID | 1360685813.13993.12.camel@edumazet-glaptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 02/12/2013 08:16 PM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Tommi was fuzzing with trinity and reported the following problem : > > commit 3f518bf745 (datagram: Add offset argument to __skb_recv_datagram) > missed that a raw socket receive queue can contain skbs with no payload. > > We can loop in __skb_recv_datagram() with MSG_PEEK mode, because > wait_for_packet() is not prepared to skip these skbs. > > [ 83.541011] INFO: rcu_sched detected stalls on CPUs/tasks: {} > (detected by 0, t=26002 jiffies, g=27673, c=27672, q=75) > [ 83.541011] INFO: Stall ended before state dump start > [ 108.067010] BUG: soft lockup - CPU#0 stuck for 22s! [trinity-child31:2847] > ... > [ 108.067010] Call Trace: > [ 108.067010] [<ffffffff818cc103>] __skb_recv_datagram+0x1a3/0x3b0 > [ 108.067010] [<ffffffff818cc33d>] skb_recv_datagram+0x2d/0x30 > [ 108.067010] [<ffffffff819ed43d>] rawv6_recvmsg+0xad/0x240 > [ 108.067010] [<ffffffff818c4b04>] sock_common_recvmsg+0x34/0x50 > [ 108.067010] [<ffffffff818bc8ec>] sock_recvmsg+0xbc/0xf0 > [ 108.067010] [<ffffffff818bf31e>] sys_recvfrom+0xde/0x150 > [ 108.067010] [<ffffffff81ca4329>] system_call_fastpath+0x16/0x1b > > Reported-by: Tommi Rantala <tt.rantala@gmail.com> > Tested-by: Tommi Rantala <tt.rantala@gmail.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Pavel Emelyanov <xemul@parallels.com> Acked-by: Pavel Emelyanov <xemul@parallels.com> Thanks! > --- > net/core/datagram.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/datagram.c b/net/core/datagram.c > index 0337e2b..368f9c3 100644 > --- a/net/core/datagram.c > +++ b/net/core/datagram.c > @@ -187,7 +187,7 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags, > skb_queue_walk(queue, skb) { > *peeked = skb->peeked; > if (flags & MSG_PEEK) { > - if (*off >= skb->len) { > + if (*off >= skb->len && skb->len) { > *off -= skb->len; > continue; > } > > > . > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 12 Feb 2013 08:16:53 -0800 > From: Eric Dumazet <edumazet@google.com> > > Tommi was fuzzing with trinity and reported the following problem : > > commit 3f518bf745 (datagram: Add offset argument to __skb_recv_datagram) > missed that a raw socket receive queue can contain skbs with no payload. > > We can loop in __skb_recv_datagram() with MSG_PEEK mode, because > wait_for_packet() is not prepared to skip these skbs. ... > Reported-by: Tommi Rantala <tt.rantala@gmail.com> > Tested-by: Tommi Rantala <tt.rantala@gmail.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 12, 2013 at 04:07:33PM -0500, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Tue, 12 Feb 2013 08:16:53 -0800 > > > From: Eric Dumazet <edumazet@google.com> > > > > Tommi was fuzzing with trinity and reported the following problem : > > > > commit 3f518bf745 (datagram: Add offset argument to __skb_recv_datagram) > > missed that a raw socket receive queue can contain skbs with no payload. > > > > We can loop in __skb_recv_datagram() with MSG_PEEK mode, because > > wait_for_packet() is not prepared to skip these skbs. > ... > > Reported-by: Tommi Rantala <tt.rantala@gmail.com> > > Tested-by: Tommi Rantala <tt.rantala@gmail.com> > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > Applied, thanks. This issue got a CVE: http://seclists.org/oss-sec/2013/q1/310 Perhaps it's something that should go to stable? Thanks, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-02-15 at 13:41 +0100, Hannes Frederic Sowa wrote: > On Tue, Feb 12, 2013 at 04:07:33PM -0500, David Miller wrote: > > From: Eric Dumazet <eric.dumazet@gmail.com> > > Date: Tue, 12 Feb 2013 08:16:53 -0800 > > > > > From: Eric Dumazet <edumazet@google.com> > > > > > > Tommi was fuzzing with trinity and reported the following problem : > > > > > > commit 3f518bf745 (datagram: Add offset argument to __skb_recv_datagram) > > > missed that a raw socket receive queue can contain skbs with no payload. > > > > > > We can loop in __skb_recv_datagram() with MSG_PEEK mode, because > > > wait_for_packet() is not prepared to skip these skbs. > > ... > > > Reported-by: Tommi Rantala <tt.rantala@gmail.com> > > > Tested-by: Tommi Rantala <tt.rantala@gmail.com> > > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > > > Applied, thanks. > > This issue got a CVE: http://seclists.org/oss-sec/2013/q1/310 > Perhaps it's something that should go to stable? David has already worked that out for himself: http://patchwork.ozlabs.org/bundle/davem/stable/?state=* Ben.
On Fri, Feb 15, 2013 at 05:43:30PM +0000, Ben Hutchings wrote: > David has already worked that out for himself: > http://patchwork.ozlabs.org/bundle/davem/stable/?state=* Thanks, didn't know where to look. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Hannes Frederic Sowa <hannes@stressinduktion.org> Date: Fri, 15 Feb 2013 13:41:41 +0100 > On Tue, Feb 12, 2013 at 04:07:33PM -0500, David Miller wrote: >> From: Eric Dumazet <eric.dumazet@gmail.com> >> Date: Tue, 12 Feb 2013 08:16:53 -0800 >> >> > From: Eric Dumazet <edumazet@google.com> >> > >> > Tommi was fuzzing with trinity and reported the following problem : >> > >> > commit 3f518bf745 (datagram: Add offset argument to __skb_recv_datagram) >> > missed that a raw socket receive queue can contain skbs with no payload. >> > >> > We can loop in __skb_recv_datagram() with MSG_PEEK mode, because >> > wait_for_packet() is not prepared to skip these skbs. >> ... >> > Reported-by: Tommi Rantala <tt.rantala@gmail.com> >> > Tested-by: Tommi Rantala <tt.rantala@gmail.com> >> > Signed-off-by: Eric Dumazet <edumazet@google.com> >> >> Applied, thanks. > > This issue got a CVE: http://seclists.org/oss-sec/2013/q1/310 > Perhaps it's something that should go to stable? It's already queued up in my -stable queue. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/datagram.c b/net/core/datagram.c index 0337e2b..368f9c3 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -187,7 +187,7 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags, skb_queue_walk(queue, skb) { *peeked = skb->peeked; if (flags & MSG_PEEK) { - if (*off >= skb->len) { + if (*off >= skb->len && skb->len) { *off -= skb->len; continue; }