From patchwork Tue Jan 17 21:21:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 9521947 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4D7C66020A for ; Tue, 17 Jan 2017 21:25:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3F52E285EA for ; Tue, 17 Jan 2017 21:25:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 321B4285ED; Tue, 17 Jan 2017 21:25:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9A33285EA for ; Tue, 17 Jan 2017 21:25:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751212AbdAQVY1 (ORCPT ); Tue, 17 Jan 2017 16:24:27 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34210 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750888AbdAQVWy (ORCPT ); Tue, 17 Jan 2017 16:22:54 -0500 Received: by mail-wm0-f65.google.com with SMTP id c85so8995639wmi.1; Tue, 17 Jan 2017 13:22:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=A3fAtZMR3/8rzSVqnTsac+6K0CdqbmODkrJmKL3sX9o=; b=G3bqc4l+XGfpk9R65vf0po4OpTwKJb+Lt10N6IQR32CRG5mmBhlsmcuXSUhrLMZKso tm/3yE6OsBSlEPr40q6JgZzkBIarm4yrCV33ikV1qsCAxScRqWyIpQffUAgsXozE6m7A V/gQjAySg2xd1VcjG2BmJubGOWKkDP5z8o8qCh1sPwAhURITpYbkihtLJVpml2LjWbFD tDRVbC3i90f6JjIlnPWJSL1UbE6L3ygJ1lUM6f43RZ5nc9UvBzUGPAqjZVF6tium5te1 bFXt7CyG0EXrk4IZjwUr370W9k8XN6oPh4VruzhO8KUwfDMeTYtn/5lK7Lif/amoQ2QO IEqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=A3fAtZMR3/8rzSVqnTsac+6K0CdqbmODkrJmKL3sX9o=; b=RFxROmCRJCsSC9//3yHAwQSIMZPex1EmshTRSf898exfSx98DE2IqabZUKGdVSRQSw BVpm1Rw1gWlbcrEY4qCw/2a/3E3M7mxhYPsFXZWk5NmjgmmeztHIYVJFi3Ni9IB5Uz2w jehrHz+Kvw/vIUf2zCqD416Tk9+kdJ6NUa/u96tXcD4HjgRf+Jc//X3v0ulYYKJT6Tn/ +GMAjO7QFFeCoxsWMlGXbFD3WTAjJwkSiVmjXXMGWs0jm0jcD3mho1DmnAy4qvbKRf6+ cB0/i57ezw8XJM523Dz6wOIt5VAnzQQjyQXZxYDl1kC9umjNu10wsrn+eNO7zIz5o+zk pA8Q== X-Gm-Message-State: AIkVDXKisxb5xtYqHFmgJT+gbwYcdQguPHeQ8PEFr1n5VPnDtmj2xVHc+IKicbeqcvlPK/3V6doW5imhyOJebQ== X-Received: by 10.28.29.141 with SMTP id d135mr216725wmd.108.1484688129361; Tue, 17 Jan 2017 13:22:09 -0800 (PST) MIME-Version: 1.0 Received: by 10.80.164.146 with HTTP; Tue, 17 Jan 2017 13:21:48 -0800 (PST) In-Reply-To: References: <20161209013208.GW1555@ZenIV.linux.org.uk> <20161209064144.GZ1555@ZenIV.linux.org.uk> From: Cong Wang Date: Tue, 17 Jan 2017 13:21:48 -0800 Message-ID: Subject: Re: fs, net: deadlock between bind/splice on af_unix To: Dmitry Vyukov Cc: Al Viro , "linux-fsdevel@vger.kernel.org" , LKML , David Miller , Rainer Weikusat , Hannes Frederic Sowa , netdev , Eric Dumazet , syzkaller Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Thanks. diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 127656e..5d4b4d1 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -995,6 +995,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) unsigned int hash; struct unix_address *addr; struct hlist_head *list; + struct path path; err = -EINVAL; if (sunaddr->sun_family != AF_UNIX) @@ -1010,9 +1011,20 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) goto out; addr_len = err; + if (sun_path[0]) { + umode_t mode = S_IFSOCK | + (SOCK_INODE(sock)->i_mode & ~current_umask()); + err = unix_mknod(sun_path, mode, &path); + if (err) { + if (err == -EEXIST) + err = -EADDRINUSE; + goto out; + } + } + err = mutex_lock_interruptible(&u->bindlock); if (err) - goto out; + goto out_put; err = -EINVAL; if (u->addr) @@ -1029,16 +1041,6 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) atomic_set(&addr->refcnt, 1); if (sun_path[0]) { - struct path path; - umode_t mode = S_IFSOCK | - (SOCK_INODE(sock)->i_mode & ~current_umask()); - err = unix_mknod(sun_path, mode, &path); - if (err) { - if (err == -EEXIST) - err = -EADDRINUSE; - unix_release_addr(addr); - goto out_up; - } addr->hash = UNIX_HASH_SIZE; hash = d_backing_inode(path.dentry)->i_ino & (UNIX_HASH_SIZE - 1); spin_lock(&unix_table_lock); @@ -1065,6 +1067,9 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) spin_unlock(&unix_table_lock); out_up: mutex_unlock(&u->bindlock); +out_put: + if (err) + path_put(&path); out: return err; }