[PATCH net] net: datagram: fix unbounded loop in __skb_try_recv_datagram()

[PATCH net] net: datagram: fix unbounded loop in __skb_try_recv_datagram()

[Paolo Abeni]

Christoph reported a stall while peeking datagram with an offset when
busy polling is enabled. __skb_try_recv_datagram() uses as the loop
termination condition 'queue empty'. When peeking, the socket
queue can be not empty, even when no additional packets are received.

Address the issue explicitly checking for receive queue changes,
as currently done by __skb_wait_for_more_packets().

Fixes: 2b5cd0dfa384 ("net: Change return type of sk_busy_loop from bool to void")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
--
Notes:
- the actual bug is older than the reported 'Fixes' tag, but this
  patch is relevant/expected to work only since said commit.
- the termination condition of the busy look still looks incorrect,
  but I'm unable to trigger any bad behavior on top of this.
---
 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 b2651bb6d2a3..e657289db4ac 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -279,7 +279,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
 			break;
 
 		sk_busy_loop(sk, flags & MSG_DONTWAIT);
-	} while (!skb_queue_empty(&sk->sk_receive_queue));
+	} while (sk->sk_receive_queue.prev != *last);
 
 	error = -EAGAIN;
 
-- 
2.20.1

Re: [PATCH net] net: datagram: fix unbounded loop in __skb_try_recv_datagram()

[David Miller]

From: Paolo Abeni <pabeni@redhat.com>
Date: Mon, 25 Mar 2019 14:18:06 +0100

> Christoph reported a stall while peeking datagram with an offset when
> busy polling is enabled. __skb_try_recv_datagram() uses as the loop
> termination condition 'queue empty'. When peeking, the socket
> queue can be not empty, even when no additional packets are received.
> 
> Address the issue explicitly checking for receive queue changes,
> as currently done by __skb_wait_for_more_packets().
> 
> Fixes: 2b5cd0dfa384 ("net: Change return type of sk_busy_loop from bool to void")
> Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied and queued up for -stable, thanks Paolo.