Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752496AbbKYWwf (ORCPT ); Wed, 25 Nov 2015 17:52:35 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:38994 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751575AbbKYWwb (ORCPT ); Wed, 25 Nov 2015 17:52:31 -0500 Message-Id: <1448491950.1848115.450243417.726E2DCB@webmail.messagingengine.com> X-Sasl-Enc: nRMPC8NOd5K15kqIZ43pdfA76iJXJn/0HzgJaNp+N7tt 1448491950 From: Hannes Frederic Sowa To: Eric Dumazet Cc: Rainer Weikusat , Eric Dumazet , Dmitry Vyukov , Benjamin LaHaise , "David S. Miller" , Al Viro , David Howells , Ying Xue , "Eric W. Biederman" , netdev , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain X-Mailer: MessagingEngine.com Webmail Interface - ajax-227d657c In-Reply-To: <1448491414.24696.60.camel@edumazet-glaptop2.roam.corp.google.com> References: <87poyzj7j2.fsf@doppelsaurus.mobileactivedefense.com> <87io4qevdp.fsf@doppelsaurus.mobileactivedefense.com> <87io4q3u8u.fsf@doppelsaurus.mobileactivedefense.com> <1448471494.24696.18.camel@edumazet-glaptop2.roam.corp.google.com> <87a8q23s2a.fsf@doppelsaurus.mobileactivedefense.com> <1448473891.24696.21.camel@edumazet-glaptop2.roam.corp.google.com> <87610q3pjg.fsf@doppelsaurus.mobileactivedefense.com> <1448476744.24696.25.camel@edumazet-glaptop2.roam.corp.google.com> <87y4dl3m5c.fsf@doppelsaurus.mobileactivedefense.com> <1448481002.24696.30.camel@edumazet-glaptop2.roam.corp.google.com> <1448483017.24696.33.camel@edumazet-glaptop2.roam.corp.google.com> <87two93ig8.fsf@doppelsaurus.mobileactivedefense.com> <1448489350.24696.47.camel@edumazet-glaptop2.roam.corp.google.com> <1448490732.1842763.450231537.5358AF37@webmail.messagingengine.com> <1448491414.24696.60.camel@edumazet-glaptop2.roam.corp.google.com> Subject: Re: use-after-free in sock_wake_async Date: Wed, 25 Nov 2015 23:52:30 +0100 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2794 Lines: 78 On Wed, Nov 25, 2015, at 23:43, Eric Dumazet wrote: > On Wed, 2015-11-25 at 23:32 +0100, Hannes Frederic Sowa wrote: > > On Wed, Nov 25, 2015, at 23:09, Eric Dumazet wrote: > > > On Wed, 2015-11-25 at 20:57 +0000, Rainer Weikusat wrote: > > > > > > > I do agree that keeping the ->sk_data_ready outside of the lock will > > > > very likely have performance advantages. That's just something I > > > > wouldn't have undertaken because I'd be reluctant to make a fairly > > > > complicated change to a lot of code. > > > > > > All I am saying is that we can keep current performance. > > > > > > We already have the core infrastructure, we only need to properly use > > > it. > > > > > > I will split my changes in two parts. > > > > > > One part doing a very boring change of > > > > > > rename SOCK_ASYNC_NOSPACE and SOCK_ASYNC_WAITDATA > > > for X in SOCK_ASYNC_NOSPACE SOCK_ASYNC_WAITDATA > > > > > > set_bit(X, &sk->sk_socket->flags) -> sk_set_bit(X, sk) > > > clear_bit(X, &sk->sk_socket->flags) -> sk_clear_bit(X, sk) > > > > sk_set_bit and sk_clear_bit will forward the set_bit and clear_bit into > > the socket_wq like you explained above? > > In the first patch (no functional change), the helpers will look like > > static void inline sk_set_bit(int nr, struct sock *sk) > { > set_bit(nr, &sk->sk_socket->flags); > } > > > Then the second patch will change the helper to : > > static void inline sk_set_bit(int nr, struct sock *sk) > { > set_bit(nr, &sk->sk_wq_raw->flags); > } Yep, that looks sensible. > > > The rename will help backports to catch code that might have been > > > removed in recent kernels. > > > > > > Then the second patch will do the actual changes, and they will look > > > very sensible for people wanting to review them, and or familiar with > > > the stack, do not worry ;) > > > > Do you see a chance to inline socket_wq into struct socket and discard > > struct socket_alloc in one go by rcu in socket_destroy_inode? > > ??? > > I guess you missed the whole point to have socket_wq allocated outside > of the inode :( Yep, sure, so inode could be torn down while wq is freed by rcu callback. > inodes are not rcu protected (yet). I certainly don't want to mess with > VFS, we have enough problems in net/ directory already. I have seen filesystems already doing so in .destroy_inode, that's why I am asking. The allocation happens the same way as we do with sock_alloc, e.g. shmem. I actually thought that struct inode already provides an rcu_head for exactly that reason. Bye, Hannes -- 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/