From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 841EDC3A589 for ; Thu, 15 Aug 2019 20:23:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5DC272083B for ; Thu, 15 Aug 2019 20:23:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731607AbfHOUXz (ORCPT ); Thu, 15 Aug 2019 16:23:55 -0400 Received: from fieldses.org ([173.255.197.46]:34802 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728579AbfHOUXz (ORCPT ); Thu, 15 Aug 2019 16:23:55 -0400 Received: by fieldses.org (Postfix, from userid 2815) id 27271BC3; Thu, 15 Aug 2019 16:23:55 -0400 (EDT) Date: Thu, 15 Aug 2019 16:23:55 -0400 To: Al Viro Cc: Tetsuo Handa , linux-fsdevel@vger.kernel.org, "J. Bruce Fields" , syzbot Subject: Re: [PATCH] nfsd: fix dentry leak upon mkdir failure. Message-ID: <20190815202355.GC19554@fieldses.org> References: <0000000000001097b6058fe1fb22@google.com> <1565576171-6586-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp> <20190812030354.GR1131@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190812030354.GR1131@ZenIV.linux.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) From: bfields@fieldses.org (J. Bruce Fields) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Mon, Aug 12, 2019 at 04:03:54AM +0100, Al Viro wrote: > On Mon, Aug 12, 2019 at 11:16:11AM +0900, Tetsuo Handa wrote: > > syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by > > d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault > > injection) [1]. > > That's not the only problem I see there. > ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600); > if (ret) > goto out_err; > if (ncl) { > d_inode(dentry)->i_private = ncl; > kref_get(&ncl->cl_ref); > } > and we are doing that to inode *after* it has become visible via dcache - > __nfsd_mkdir() has already done d_add() (and pinged f-snotify, at that). > Looks fishy... Whoops, thanks. Considering the following (untested). --b. commit 930f7dd94cc2 Author: J. Bruce Fields Date: Thu Aug 15 16:18:26 2019 -0400 nfsd: initialize i_private before d_add A process could race in an open and attempt to read one of these files before i_private is initialized, and get a spurious error. Reported-by: Al Viro Signed-off-by: J. Bruce Fields diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index b14f825c62fe..3cf4f6aa48d6 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1171,13 +1171,17 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode) return inode; } -static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) +static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode, struct nfsdfs_client *ncl) { struct inode *inode; inode = nfsd_get_inode(dir->i_sb, mode); if (!inode) return -ENOMEM; + if (ncl) { + inode->i_private = ncl; + kref_get(&ncl->cl_ref); + } d_add(dentry, inode); inc_nlink(dir); fsnotify_mkdir(dir, dentry); @@ -1194,13 +1198,9 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc dentry = d_alloc_name(parent, name); if (!dentry) goto out_err; - ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600); + ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600, ncl); if (ret) goto out_err; - if (ncl) { - d_inode(dentry)->i_private = ncl; - kref_get(&ncl->cl_ref); - } out: inode_unlock(dir); return dentry;