From patchwork Fri Jan 20 04:57:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 9527369 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 E280060113 for ; Fri, 20 Jan 2017 04:58:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D2D6828664 for ; Fri, 20 Jan 2017 04:58:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C797528685; Fri, 20 Jan 2017 04:58:30 +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 4262928664 for ; Fri, 20 Jan 2017 04:58:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751250AbdATE6A (ORCPT ); Thu, 19 Jan 2017 23:58:00 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36325 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751172AbdATE56 (ORCPT ); Thu, 19 Jan 2017 23:57:58 -0500 Received: by mail-wm0-f67.google.com with SMTP id r126so3864654wmr.3; Thu, 19 Jan 2017 20:57:57 -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=zrcyWjXu0+8vFSdm+zRvff5TyESQ0k0Zym39KUSJh4U=; b=JZe+ooDnIe4odUPfZuDKZCPOjaSPWpN+6mo5zye4jVP88zcqtORgR04Li2nZeJVmWR mAyUon+pSJRPbsQ5t0SI5C0mZDOtHJ1Jj/2q9vX4tcdVxusqzcGWzuy1YgOZXWWUAQ1P kFEOrOOE+HrdgMGeSZAAvqdVPHTyWJmHunQh0UJfehOekVZiUHkv+lcTo5XKSLLLj9/q g2lRSe+jcLrVMfhoCk29uDvOQhPxryWn9MguRdWQRI7IZx1Z4AVwLR0W/wkceCVIxWh8 1piybMrGI2roJ0VGy/B6uqNltBWZGU1YJXfupKE5PISHu85IUF6WB1BXOnUjOf02cNOp wbNA== 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=zrcyWjXu0+8vFSdm+zRvff5TyESQ0k0Zym39KUSJh4U=; b=pegJlmUmFCZQ56jiX0EqZXHlGorREdUsHIM16D4qEnM/p0js2VjoelLCvOnNX32LMV YP9nfxokERT2sJCRr/nWo6UJwLBHl20AX8eDqEjvVT4o2NDASTY7EwiNd7DjepVLeUiF hV0vhuPTo/WQOOiL53HnePdZ6C5f+MNSU921by8kUM4/JvWIK9FMze+vyxOZxnbbTTTQ zYcoG5hCUXC9z4GMiaf4/X8qpTH1XoU/6CPqFC22rkd+zw7hYzRfYYaLtWFlgQUm0wHz 8PuFxtZVdu6KV9/x9TsqdftrSZYXiqqGJawJyMRcsxkdMx0OdggcAlStStmi/vnVAsaC EFjw== X-Gm-Message-State: AIkVDXLL85zeKwHyTiGRfJ0mjsQTQmI5QhRflbsQ1DYGpGftCjf61lXg7opmaFbaFc/yfxNjnVBDG17qnMUedg== X-Received: by 10.28.11.10 with SMTP id 10mr1547936wml.109.1484888276799; Thu, 19 Jan 2017 20:57:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.80.164.146 with HTTP; Thu, 19 Jan 2017 20:57:36 -0800 (PST) In-Reply-To: References: <20161209013208.GW1555@ZenIV.linux.org.uk> <20161209064144.GZ1555@ZenIV.linux.org.uk> From: Cong Wang Date: Thu, 19 Jan 2017 20:57:36 -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 Wed, Jan 18, 2017 at 1:17 AM, Dmitry Vyukov wrote: > On Tue, Jan 17, 2017 at 10:21 PM, 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 instantly hit: > Oh, sorry about it, I forgot to initialize struct path... Attached is the updated version, I just did a boot test, no crash at least. ;) Thanks! Tested-by: Dmitry Vyukov diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 127656e..cef7987 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 = { NULL, NULL }; 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; }