From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753648AbdAZXih (ORCPT ); Thu, 26 Jan 2017 18:38:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55508 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753588AbdAZXif (ORCPT ); Thu, 26 Jan 2017 18:38:35 -0500 Date: Fri, 27 Jan 2017 00:29:12 +0100 From: Mateusz Guzik To: Cong Wang Cc: Dmitry Vyukov , Al Viro , "linux-fsdevel@vger.kernel.org" , LKML , David Miller , Rainer Weikusat , Hannes Frederic Sowa , netdev , Eric Dumazet , syzkaller Subject: Re: fs, net: deadlock between bind/splice on af_unix Message-ID: <20170126232912.rgh6undqxd4tofqj@dhcp-1-212.brq.redhat.com> References: <20161209013208.GW1555@ZenIV.linux.org.uk> <20161209064144.GZ1555@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 26 Jan 2017 23:29:27 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 17, 2017 at 01:21:48PM -0800, Cong Wang wrote: > On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov wrote: > > On Fri, Dec 9, 2016 at 7:41 AM, Al Viro wrote: > >> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote: > >> > >>> > Why do we do autobind there, anyway, and why is it conditional on > >>> > SOCK_PASSCRED? Note that e.g. for SOCK_STREAM we can bloody well get > >>> > to sending stuff without autobind ever done - just use socketpair() > >>> > to create that sucker and we won't be going through the connect() > >>> > at all. > >>> > >>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(), > >>> not SOCK_STREAM. > >> > >> Yes, I've noticed. What I'm asking is what in there needs autobind triggered > >> on sendmsg and why doesn't the same need affect the SOCK_STREAM case? > >> > >>> I guess some lock, perhaps the u->bindlock could be dropped before > >>> acquiring the next one (sb_writer), but I need to double check. > >> > >> Bad idea, IMO - do you *want* autobind being able to come through while > >> bind(2) is busy with mknod? > > > > > > Ping. This is still happening on HEAD. > > > > Thanks for your reminder. Mind to give the attached patch (compile only) > a try? I take another approach to fix this deadlock, which moves the > unix_mknod() out of unix->bindlock. Not sure if there is any unexpected > impact with this way. > I don't think this is the right approach. Currently the file creation is potponed until unix_bind can no longer fail otherwise. With it reordered, it may be someone races you with a different path and now you are left with a file to clean up. Except it is quite unclear for me if you can unlink it. I don't have a good idea how to fix it. A somewhat typical approach would introduce an intermediate state ("under construction") and drop the lock between calling into unix_mknod. In this particular case, perhaps you could repurpose gc_flags as a general flags carrier and add a 'binding in process' flag to test.