Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: [PATCH RFC] NFSD: Bump the ref count on nf_inode
From:   Chuck Lever <chuck.lever@oracle.com>
To:     jlayton@redhat.com
Cc:     linux-nfs@vger.kernel.org
Date:   Thu, 07 Jul 2022 11:58:30 -0400
Message-ID: <165720933938.1180.14325183467695610136.stgit@klimt.1015granger.net>
User-Agent: StGit/1.5.dev3+g9561319
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

The documenting comment for struct nf_file states:

/*
 * A representation of a file that has been opened by knfsd. These are hashed
 * in the hashtable by inode pointer value. Note that this object doesn't
 * hold a reference to the inode by itself, so the nf_inode pointer should
 * never be dereferenced, only used for comparison.
 */

However, nfsd_file_mark_find_or_create() does dereference the pointer stored
in this field.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/filecache.c |    3 +++
 fs/nfsd/filecache.h |    4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

Hi Jeff-

I'm still testing this one, but I'm wondering what you think of it.
I did hit a KASAN splat that might be related, but it's not 100%
reproducible.


diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 9cb2d590c036..7b43bb427a53 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -180,6 +180,7 @@ nfsd_file_alloc(struct inode *inode, unsigned int may, unsigned int hashval,
 		nf->nf_cred = get_current_cred();
 		nf->nf_net = net;
 		nf->nf_flags = 0;
+		ihold(inode);
 		nf->nf_inode = inode;
 		nf->nf_hashval = hashval;
 		refcount_set(&nf->nf_ref, 1);
@@ -210,6 +211,7 @@ nfsd_file_free(struct nfsd_file *nf)
 		fput(nf->nf_file);
 		flush = true;
 	}
+	iput(nf->nf_inode);
 	call_rcu(&nf->nf_rcu, nfsd_file_slab_free);
 	return flush;
 }
@@ -940,6 +942,7 @@ nfsd_do_file_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (nf == NULL)
 		goto open_file;
 	spin_unlock(&nfsd_file_hashtbl[hashval].nfb_lock);
+	iput(new->nf_inode);
 	nfsd_file_slab_free(&new->nf_rcu);
 
 wait_for_construction:
diff --git a/fs/nfsd/filecache.h b/fs/nfsd/filecache.h
index 1da0c79a5580..01fbf6e88cce 100644
--- a/fs/nfsd/filecache.h
+++ b/fs/nfsd/filecache.h
@@ -24,9 +24,7 @@ struct nfsd_file_mark {
 
 /*
  * A representation of a file that has been opened by knfsd. These are hashed
- * in the hashtable by inode pointer value. Note that this object doesn't
- * hold a reference to the inode by itself, so the nf_inode pointer should
- * never be dereferenced, only used for comparison.
+ * in the hashtable by inode pointer value.
  */
 struct nfsd_file {
 	struct hlist_node	nf_node;