Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755111AbbKXVpQ (ORCPT ); Tue, 24 Nov 2015 16:45:16 -0500 Received: from kanga.kvack.org ([205.233.56.17]:46572 "EHLO kanga.kvack.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754398AbbKXVpN (ORCPT ); Tue, 24 Nov 2015 16:45:13 -0500 Date: Tue, 24 Nov 2015 16:45:12 -0500 From: Benjamin LaHaise To: Jason Baron Cc: Eric Dumazet , Dmitry Vyukov , "David S. Miller" , Hannes Frederic Sowa , Al Viro , David Howells , Ying Xue , "Eric W. Biederman" , Rainer Weikusat , netdev , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin Subject: Re: use-after-free in sock_wake_async Message-ID: <20151124214512.GM27046@kvack.org> References: <5654D6D9.1050108@akamai.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5654D6D9.1050108@akamai.com> User-Agent: Mutt/1.4.2.2i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2600 Lines: 91 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/