tcp: fix recv with flags MSG_WAITALL | MSG_PEEK
Subsystems (5)
PATCH 1/1 tcp: fix recv with flags MSG_WAITALL | MSG_PEEK
Currently, tcp_recvmsg enters a busy loop in sk_wait_data if called with flags = MSG_WAITALL | MSG_PEEK.
sk_wait_data waits for sk_receive_queue not empty, but in this case, the receive queue is not empty, but does not contain any skb that we can use.
Add a "last skb seen on receive queue" argument to sk_wait_data, so that it sleeps until the receive queue has new skbs.
--- include/net/sock.h | 2 +- net/core/sock.c | 5 +++-- net/dccp/proto.c | 2 +- net/ipv4/tcp.c | 11 +++++++---- net/llc/af_llc.c | 4 ++-- 5 files changed, 14 insertions(+), 10 deletions(-)
include/net/sock.h +1 -1
@@ -902,7 +902,7 @@ void sk_stream_kill_queues(struct sock *sk);void sk_set_memalloc (struct sock *sk);void sk_clear_memalloc (struct sock *sk); -int sk_wait_data (struct sock *sk,long *timeo); +int sk_wait_data (struct sock *sk,long *timeo,const struct sk_buff *skb);struct request_sock_ops ;struct timewait_sock_ops ;
net/core/sock.c +3 -2
@@ -1967,20 +1967,21 @@ static void __release_sock(struct sock *sk)* sk_wait_data - wait for data to arrive at sk_receive_queue * @sk: sock to wait on * @timeo: for how long +* @skb: last skb seen on sk_receive_queue * * Now socket state including sk->sk_err is changed only under lock, * hence we may omit checks after joining wait queue. * We check receive queue before schedule() only as optimization; * it is very likely that release_sock() added new data. */ -int sk_wait_data (struct sock *sk,long *timeo) +int sk_wait_data (struct sock *sk,long *timeo,const struct sk_buff *skb) {int rc;DEFINE_WAIT (wait);prepare_to_wait (sk_sleep (sk), &wait,TASK_INTERRUPTIBLE );set_bit (SOCK_ASYNC_WAITDATA , &sk->sk_socket->flags); - rc =sk_wait_event (sk, timeo, !skb_queue_empty (&sk->sk_receive_queue)); + rc =sk_wait_event (sk, timeo,skb_peek_tail (&sk->sk_receive_queue) != skb);clear_bit (SOCK_ASYNC_WAITDATA , &sk->sk_socket->flags);finish_wait (sk_sleep (sk), &wait);return rc;
net/dccp/proto.c +1 -1
@@ -886,7 +886,7 @@ verify_sock_status:break ; } -sk_wait_data (sk, &timeo); +sk_wait_data (sk, &timeo,NULL );continue ; found_ok_skb:if (len > skb->len)
net/ipv4/tcp.c +7 -4
@@ -780,7 +780,7 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos, ret = -EAGAIN ;break ; } -sk_wait_data (sk, &timeo); +sk_wait_data (sk, &timeo,NULL );if (signal_pending (current)) { ret =sock_intr_errno (timeo);break ;
@@ -1575,7 +1575,7 @@ int tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock,int target;/* Read at least this many bytes */ long timeo;struct task_struct *user_recv =NULL ; -struct sk_buff *skb; +struct sk_buff *skb, *last;u32 urg_hole =0 ;if (unlikely (flags &MSG_ERRQUEUE ))
@@ -1635,7 +1635,9 @@ int tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock,/* Next get a buffer. */ + last =skb_peek_tail (&sk->sk_receive_queue); skb_queue_walk(&sk->sk_receive_queue, skb) { + last = skb;/* Now that we have two receive queues this * shouldn't happen. */
@@ -1754,8 +1756,9 @@ int tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock,/* Do not sleep, just process backlog. */ release_sock (sk);lock_sock (sk); - }else -sk_wait_data (sk, &timeo); + }else { +sk_wait_data (sk, &timeo, last); + }if (user_recv) {int chunk;
net/llc/af_llc.c +2 -2
@@ -613,7 +613,7 @@ static int llc_wait_data(struct sock *sk, long timeo)if (signal_pending (current))break ; rc =0 ; -if (sk_wait_data (sk, &timeo)) +if (sk_wait_data (sk, &timeo,NULL ))break ; }return rc;
@@ -802,7 +802,7 @@ static int llc_ui_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,release_sock (sk);lock_sock (sk); }else -sk_wait_data (sk, &timeo); +sk_wait_data (sk, &timeo,NULL );if ((flags &MSG_PEEK ) && peek_seq != llc->copied_seq) { net_dbg_ratelimited("LLC(%s:%d): Application bug, race in MSG_PEEK\n",
Signature · 1 line
2.4.6
Quoted 10 lines · Sabrina Dubroca
Currently,
tcp_recvmsgenters a busy loop insk_wait_dataif called with flags =MSG_WAITALL|MSG_PEEK.
sk_wait_datawaits forsk_receive_queuenot empty, but in this case, the receive queue is not empty, but does not contain any skb that we can use.Add a "last skb seen on receive queue" argument to
sk_wait_data, so that it sleeps until the receive queue has new skbs.
Quoted 7 lines
Reported-by Enrico Scholz <[email protected]>Reported-by Dan Searle <[email protected]>Signed-off-by Sabrina Dubroca <[email protected]>---
Very nice !
From: Sabrina Dubroca <[email protected]>
Date: Fri, 24 Jul 2015 18:19:25 +0200
Quoted 10 lines
Currently,
tcp_recvmsgenters a busy loop insk_wait_dataif called with flags =MSG_WAITALL|MSG_PEEK.
sk_wait_datawaits forsk_receive_queuenot empty, but in this case, the receive queue is not empty, but does not contain any skb that we can use.Add a "last skb seen on receive queue" argument to
sk_wait_data, so that it sleeps until the receive queue has new skbs.
Quoted 6 lines
Reported-by Enrico Scholz <[email protected]>Reported-by Dan Searle <[email protected]>Signed-off-by Sabrina Dubroca <[email protected]>
Applied, thanks.