Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1276778
| From | Benjamin LaHaise <bcrl@kvack.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: use-after-free in sock_wake_async |
| Date | 2015-11-24 22:50 +0100 |
| Message-ID | <qytDj-5dS-9@gated-at.bofh.it> (permalink) |
| References | <qymBP-JT-15@gated-at.bofh.it> <qynHD-1tb-89@gated-at.bofh.it> <qyttF-59Z-29@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Tue, Nov 24, 2015 at 04:30:01PM -0500, Jason Baron wrote:
> So looking at this trace I think its the other->sk_socket that gets
> freed and then we call sk_wake_async() on it.
>
> We could I think grab the socket reference there with unix_state_lock(),
> since that is held by unix_release_sock() before the final iput() is called.
>
> So something like below might work (compile tested only):
That just adds the performance regression back in. It should be possible
to protect the other socket dereference using RCU. I haven't had time to
look at this yet today, but will try to find some time this evening to come
up with a suggested patch.
-ben
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index aaa0b58..2b014f1 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -196,6 +196,19 @@ static inline int unix_recvq_full(struct sock const
> *sk)
> return skb_queue_len(&sk->sk_receive_queue) > sk->sk_max_ack_backlog;
> }
>
> +struct socket *unix_peer_get_socket(struct sock *s)
> +{
> + struct socket *peer;
> +
> + unix_state_lock(s);
> + peer = s->sk_socket;
> + if (peer)
> + __iget(SOCK_INODE(s->sk_socket));
> + unix_state_unlock(s);
> +
> + return peer;
> +}
> +
> struct sock *unix_peer_get(struct sock *s)
> {
> struct sock *peer;
> @@ -1639,6 +1652,7 @@ static int unix_stream_sendmsg(struct socket
> *sock, struct msghdr *msg,
> {
> struct sock *sk = sock->sk;
> struct sock *other = NULL;
> + struct socket *other_socket = NULL;
> int err, size;
> struct sk_buff *skb;
> int sent = 0;
> @@ -1662,7 +1676,10 @@ static int unix_stream_sendmsg(struct socket
> *sock, struct msghdr *msg,
> } else {
> err = -ENOTCONN;
> other = unix_peer(sk);
> - if (!other)
> + if (other)
> + other_socket = unix_peer_get_socket(other);
> +
> + if (!other_socket)
> goto out_err;
> }
>
> @@ -1721,6 +1738,9 @@ static int unix_stream_sendmsg(struct socket
> *sock, struct msghdr *msg,
> sent += size;
> }
>
> + if (other_socket)
> + iput(SOCK_INODE(other_socket));
> +
> scm_destroy(&scm);
>
> return sent;
> @@ -1733,6 +1753,8 @@ pipe_err:
> send_sig(SIGPIPE, current, 0);
> err = -EPIPE;
> out_err:
> + if (other_socket)
> + iput(SOCK_INODE(other_socket));
> scm_destroy(&scm);
> return sent ? : err;
> }
--
"Thought is the essence of where you are now."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
use-after-free in sock_wake_async Dmitry Vyukov <dvyukov@google.com> - 2015-11-24 15:20 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <edumazet@google.com> - 2015-11-24 16:30 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-24 16:40 +0100
Re: use-after-free in sock_wake_async Jason Baron <jbaron@akamai.com> - 2015-11-24 22:40 +0100
Re: use-after-free in sock_wake_async Benjamin LaHaise <bcrl@kvack.org> - 2015-11-24 22:50 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <edumazet@google.com> - 2015-11-24 23:10 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <edumazet@google.com> - 2015-11-24 23:20 +0100
Re: use-after-free in sock_wake_async Al Viro <viro@ZenIV.linux.org.uk> - 2015-11-24 22:50 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 00:40 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <edumazet@google.com> - 2015-11-25 00:50 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 02:20 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <edumazet@google.com> - 2015-11-25 02:20 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 03:30 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 06:50 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 15:20 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 17:50 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 18:20 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 18:40 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 19:00 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 19:30 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 19:40 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 20:40 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 21:00 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 21:30 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 22:00 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 23:10 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-25 23:40 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-25 23:50 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-26 00:00 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-26 14:40 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-26 15:40 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-26 17:00 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-26 18:10 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-26 18:20 +0100
Re: use-after-free in sock_wake_async Hannes Frederic Sowa <hannes@stressinduktion.org> - 2015-11-26 18:10 +0100
Re: use-after-free in sock_wake_async Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-26 18:30 +0100
Re: use-after-free in sock_wake_async Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-11-25 02:20 +0100
csiph-web