Commit 293ad604 authored by Octavian Purdila's avatar Octavian Purdila Committed by David S. Miller

tcp: Fix for race due to temporary drop of the socket lock in skb_splice_bits.

skb_splice_bits temporary drops the socket lock while iterating over
the socket queue in order to break a reverse locking condition which
happens with sendfile. This, however, opens a window of opportunity
for tcp_collapse() to aggregate skbs and thus potentially free the
current skb used in skb_splice_bits and tcp_read_sock.

This patch fixes the problem by (re-)getting the same "logical skb"
after the lock has been temporary dropped.

Based on idea and initial patch from Evgeniy Polyakov.
Signed-off-by: default avatarOctavian Purdila <opurdila@ixiacom.com>
Acked-by: default avatarEvgeniy Polyakov <johnpol@2ka.mipt.ru>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 26af65cb
...@@ -1445,6 +1445,7 @@ int skb_splice_bits(struct sk_buff *__skb, unsigned int offset, ...@@ -1445,6 +1445,7 @@ int skb_splice_bits(struct sk_buff *__skb, unsigned int offset,
if (spd.nr_pages) { if (spd.nr_pages) {
int ret; int ret;
struct sock *sk = __skb->sk;
/* /*
* Drop the socket lock, otherwise we have reverse * Drop the socket lock, otherwise we have reverse
...@@ -1455,9 +1456,9 @@ int skb_splice_bits(struct sk_buff *__skb, unsigned int offset, ...@@ -1455,9 +1456,9 @@ int skb_splice_bits(struct sk_buff *__skb, unsigned int offset,
* we call into ->sendpage() with the i_mutex lock held * we call into ->sendpage() with the i_mutex lock held
* and networking will grab the socket lock. * and networking will grab the socket lock.
*/ */
release_sock(__skb->sk); release_sock(sk);
ret = splice_to_pipe(pipe, &spd); ret = splice_to_pipe(pipe, &spd);
lock_sock(__skb->sk); lock_sock(sk);
return ret; return ret;
} }
......
...@@ -1227,7 +1227,14 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc, ...@@ -1227,7 +1227,14 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
copied += used; copied += used;
offset += used; offset += used;
} }
if (offset != skb->len) /*
* If recv_actor drops the lock (e.g. TCP splice
* receive) the skb pointer might be invalid when
* getting here: tcp_collapse might have deleted it
* while aggregating skbs from the socket queue.
*/
skb = tcp_recv_skb(sk, seq-1, &offset);
if (!skb || (offset+1 != skb->len))
break; break;
} }
if (tcp_hdr(skb)->fin) { if (tcp_hdr(skb)->fin) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment