Return-Path: Received: from fieldses.org ([173.255.197.46]:44424 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933193AbbCYPI4 (ORCPT ); Wed, 25 Mar 2015 11:08:56 -0400 Date: Wed, 25 Mar 2015 11:08:56 -0400 From: "J. Bruce Fields" To: Jeff Layton Cc: Chengyu Song , linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, taesoo@gatech.edu, changwoo@gatech.edu, sanidhya@gatech.edu, blee@gatech.edu Subject: Re: [PATCH 1/1] nfsd: incorrect check for debugfs returns Message-ID: <20150325150856.GB13764@fieldses.org> References: <1427165885-20823-1-git-send-email-csong84@gatech.edu> <20150324064420.515096c7@tlielax.poochiereds.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20150324064420.515096c7@tlielax.poochiereds.net> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, Mar 24, 2015 at 06:44:20AM -0400, Jeff Layton wrote: > On Mon, 23 Mar 2015 22:58:05 -0400 > Chengyu Song wrote: > > > debugfs_create_dir and debugfs_create_file may return -ENODEV when debugfs > > is not configured, so the return value should be checked against ERROR_VALUE > > as well, otherwise the later dereference of the dentry pointer would crash > > the kernel. > > > > Signed-off-by: Chengyu Song > > --- > > fs/nfsd/fault_inject.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c > > index c16bf5a..621d065 100644 > > --- a/fs/nfsd/fault_inject.c > > +++ b/fs/nfsd/fault_inject.c > > @@ -132,19 +132,23 @@ int nfsd_fault_inject_init(void) > > unsigned int i; > > struct nfsd_fault_inject_op *op; > > umode_t mode = S_IFREG | S_IRUSR | S_IWUSR; > > + struct dentry *dent; > > > > - debug_dir = debugfs_create_dir("nfsd", NULL); > > - if (!debug_dir) > > + dent = debugfs_create_dir("nfsd", NULL); > > + if (IS_ERR_OR_NULL(dent)) > > goto fail; > > + debug_dir = dent; > > > > for (i = 0; i < NUM_INJECT_OPS; i++) { > > op = &inject_ops[i]; > > - if (!debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd)) > > + dent = debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd); > > + if (IS_ERR_OR_NULL(dent)) > > goto fail; > > + > > } > > return 0; > > > > fail: > > nfsd_fault_inject_cleanup(); > > - return -ENOMEM; > > + return dent ? PTR_ERR(dent) : -ENOMEM; > > } > > No objection to taking this patch in the near term if it helps, but we > had discussed over the summer just removing the NFS fault injection > framework. > > Bruce, any objections to making that happen for v4.1? I was prepared to, but I think Redhat QA people told me that they do use it--which means other people may too, so I'm sort of reluctant to tear it out even if it's imperfect. --b.