Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932338AbbKYC2W (ORCPT ); Tue, 24 Nov 2015 21:28:22 -0500 Received: from mail-pa0-f51.google.com ([209.85.220.51]:35627 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754793AbbKYC2S (ORCPT ); Tue, 24 Nov 2015 21:28:18 -0500 Message-ID: <1448418495.22599.320.camel@edumazet-glaptop2.roam.corp.google.com> Subject: Re: use-after-free in sock_wake_async From: Eric Dumazet To: Eric Dumazet , Dmitry Vyukov Cc: Rainer Weikusat , Dmitry Vyukov , Benjamin LaHaise , "David S. Miller" , Hannes Frederic Sowa , Al Viro , David Howells , Ying Xue , "Eric W. Biederman" , netdev , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin Date: Tue, 24 Nov 2015 18:28:15 -0800 In-Reply-To: References: <87poyzj7j2.fsf@doppelsaurus.mobileactivedefense.com> <87io4qevdp.fsf@doppelsaurus.mobileactivedefense.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3888 Lines: 127 Dmitry, could you test following patch with your setup ? ( I tried to reproduce the error you reported but could not ) Inode can be freed (without RCU grace period), but not the socket or sk_wq By using sk_wq in the critical paths, we do not dereference the inode, Thanks ! include/linux/net.h | 2 +- include/net/sock.h | 8 ++++++-- net/core/stream.c | 2 +- net/sctp/socket.c | 6 +++++- net/socket.c | 16 +++++----------- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/linux/net.h b/include/linux/net.h index 70ac5e28e6b7..6b93ec234ce8 100644 --- a/include/linux/net.h +++ b/include/linux/net.h @@ -202,7 +202,7 @@ enum { SOCK_WAKE_URG, }; -int sock_wake_async(struct socket *sk, int how, int band); +int sock_wake_async(struct socket *sock, struct socket_wq *wq, int how, int band); int sock_register(const struct net_proto_family *fam); void sock_unregister(int family); int __sock_create(struct net *net, int family, int type, int proto, diff --git a/include/net/sock.h b/include/net/sock.h index 7f89e4ba18d1..af78f9e7a218 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -2007,8 +2007,12 @@ static inline unsigned long sock_wspace(struct sock *sk) static inline void sk_wake_async(struct sock *sk, int how, int band) { - if (sock_flag(sk, SOCK_FASYNC)) - sock_wake_async(sk->sk_socket, how, band); + if (sock_flag(sk, SOCK_FASYNC)) { + rcu_read_lock(); + sock_wake_async(sk->sk_socket, rcu_dereference(sk->sk_wq), + how, band); + rcu_read_unlock(); + } } /* Since sk_{r,w}mem_alloc sums skb->truesize, even a small frame might diff --git a/net/core/stream.c b/net/core/stream.c index d70f77a0c889..92682228919d 100644 --- a/net/core/stream.c +++ b/net/core/stream.c @@ -39,7 +39,7 @@ void sk_stream_write_space(struct sock *sk) wake_up_interruptible_poll(&wq->wait, POLLOUT | POLLWRNORM | POLLWRBAND); if (wq && wq->fasync_list && !(sk->sk_shutdown & SEND_SHUTDOWN)) - sock_wake_async(sock, SOCK_WAKE_SPACE, POLL_OUT); + sock_wake_async(sock, wq, SOCK_WAKE_SPACE, POLL_OUT); rcu_read_unlock(); } } diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 897c01c029ca..6ab04866a1e7 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -6817,9 +6817,13 @@ static void __sctp_write_space(struct sctp_association *asoc) * here by modeling from the current TCP/UDP code. * We have not tested with it yet. */ - if (!(sk->sk_shutdown & SEND_SHUTDOWN)) + if (!(sk->sk_shutdown & SEND_SHUTDOWN)) { + rcu_read_lock(); sock_wake_async(sock, + rcu_dereference(sk->sk_wq), SOCK_WAKE_SPACE, POLL_OUT); + rcu_read_unlock(); + } } } } diff --git a/net/socket.c b/net/socket.c index dd2c247c99e3..8df62c8bef90 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1058,18 +1058,12 @@ static int sock_fasync(int fd, struct file *filp, int on) /* This function may be called only under socket lock or callback_lock or rcu_lock */ -int sock_wake_async(struct socket *sock, int how, int band) +int sock_wake_async(struct socket *sock, struct socket_wq *wq, + int how, int band) { - struct socket_wq *wq; - - if (!sock) - return -1; - rcu_read_lock(); - wq = rcu_dereference(sock->wq); - if (!wq || !wq->fasync_list) { - rcu_read_unlock(); + if (!sock || !wq || !wq->fasync_list) return -1; - } + switch (how) { case SOCK_WAKE_WAITD: if (test_bit(SOCK_ASYNC_WAITDATA, &sock->flags)) @@ -1086,7 +1080,7 @@ call_kill: case SOCK_WAKE_URG: kill_fasync(&wq->fasync_list, SIGURG, band); } - rcu_read_unlock(); + return 0; } EXPORT_SYMBOL(sock_wake_async); -- 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/