| From foo@baz Fri Dec 11 11:38:35 EST 2015 |
| From: Hannes Frederic Sowa <hannes@stressinduktion.org> |
| Date: Tue, 10 Nov 2015 16:23:15 +0100 |
| Subject: af-unix: fix use-after-free with concurrent readers while splicing |
| |
| From: Hannes Frederic Sowa <hannes@stressinduktion.org> |
| |
| [ Upstream commit 73ed5d25dce0354ea381d6dc93005c3085fae03d ] |
| |
| During splicing an af-unix socket to a pipe we have to drop all |
| af-unix socket locks. While doing so we allow another reader to enter |
| unix_stream_read_generic which can read, copy and finally free another |
| skb. If exactly this skb is just in process of being spliced we get a |
| use-after-free report by kasan. |
| |
| First, we must make sure to not have a free while the skb is used during |
| the splice operation. We simply increment its use counter before unlocking |
| the reader lock. |
| |
| Stream sockets have the nice characteristic that we don't care about |
| zero length writes and they never reach the peer socket's queue. That |
| said, we can take the UNIXCB.consumed field as the indicator if the |
| skb was already freed from the socket's receive queue. If the skb was |
| fully consumed after we locked the reader side again we know it has been |
| dropped by a second reader. We indicate a short read to user space and |
| abort the current splice operation. |
| |
| This bug has been found with syzkaller |
| (http://github.com/google/syzkaller) by Dmitry Vyukov. |
| |
| Fixes: 2b514574f7e8 ("net: af_unix: implement splice for stream af_unix sockets") |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Cc: Dmitry Vyukov <dvyukov@google.com> |
| Cc: Eric Dumazet <eric.dumazet@gmail.com> |
| Acked-by: Eric Dumazet <edumazet@google.com> |
| Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/unix/af_unix.c | 18 ++++++++++++++++++ |
| 1 file changed, 18 insertions(+) |
| |
| --- a/net/unix/af_unix.c |
| +++ b/net/unix/af_unix.c |
| @@ -440,6 +440,7 @@ static void unix_release_sock(struct soc |
| if (state == TCP_LISTEN) |
| unix_release_sock(skb->sk, 1); |
| /* passed fds are erased in the kfree_skb hook */ |
| + UNIXCB(skb).consumed = skb->len; |
| kfree_skb(skb); |
| } |
| |
| @@ -2071,6 +2072,7 @@ static int unix_stream_read_generic(stru |
| |
| do { |
| int chunk; |
| + bool drop_skb; |
| struct sk_buff *skb, *last; |
| |
| unix_state_lock(sk); |
| @@ -2151,7 +2153,11 @@ unlock: |
| } |
| |
| chunk = min_t(unsigned int, unix_skb_len(skb) - skip, size); |
| + skb_get(skb); |
| chunk = state->recv_actor(skb, skip, chunk, state); |
| + drop_skb = !unix_skb_len(skb); |
| + /* skb is only safe to use if !drop_skb */ |
| + consume_skb(skb); |
| if (chunk < 0) { |
| if (copied == 0) |
| copied = -EFAULT; |
| @@ -2160,6 +2166,18 @@ unlock: |
| copied += chunk; |
| size -= chunk; |
| |
| + if (drop_skb) { |
| + /* the skb was touched by a concurrent reader; |
| + * we should not expect anything from this skb |
| + * anymore and assume it invalid - we can be |
| + * sure it was dropped from the socket queue |
| + * |
| + * let's report a short read |
| + */ |
| + err = 0; |
| + break; |
| + } |
| + |
| /* Mark read part of skb as used */ |
| if (!(flags & MSG_PEEK)) { |
| UNIXCB(skb).consumed += chunk; |