All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Lingfeng <lilingfeng3@huawei.com>
To: <chuck.lever@oracle.com>, <jlayton@kernel.org>, <neilb@suse.de>,
	<okorniev@redhat.com>, <Dai.Ngo@oracle.com>, <tom@talpey.com>,
	<linux-nfs@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <yukuai1@huaweicloud.com>, <houtao1@huawei.com>,
	<yi.zhang@huawei.com>, <yangerkun@huawei.com>,
	<lilingfeng@huaweicloud.com>, <lilingfeng3@huawei.com>
Subject: [PATCH v2] nfsd: Invoke tracking callbacks only after initialization is complete
Date: Thu, 12 Jun 2025 11:55:06 +0800	[thread overview]
Message-ID: <20250612035506.3651985-1-lilingfeng3@huawei.com> (raw)

Checking whether tracking callbacks can be called based on whether
nn->client_tracking_ops is NULL may lead to callbacks being invoked
before tracking initialization completes, causing resource access
violations (UAF, NULL pointer dereference). Examples:

1) nfsd4_client_tracking_init
   // set nn->client_tracking_ops
   nfsd4_cld_tracking_init
    nfs4_cld_state_init
     nn->reclaim_str_hashtbl = kmalloc_array
    ... // error path, goto err
    nfs4_cld_state_shutdown
     kfree(nn->reclaim_str_hashtbl)
                                      write_v4_end_grace
                                       nfsd4_end_grace
                                        nfsd4_record_grace_done
                                         nfsd4_cld_grace_done
                                          nfs4_release_reclaim
                                           nn->reclaim_str_hashtbl[i]
                                           // UAF
   // clear nn->client_tracking_ops

2) nfsd4_client_tracking_init
   // set nn->client_tracking_ops
   nfsd4_cld_tracking_init
                                      write_v4_end_grace
                                       nfsd4_end_grace
                                        nfsd4_record_grace_done
                                         nfsd4_cld_grace_done
                                          alloc_cld_upcall
                                           cn = nn->cld_net
                                           spin_lock // cn->cn_lock
                                           // NULL deref
   // error path, skip init pipe
   __nfsd4_init_cld_pipe
    cn = kzalloc
    nn->cld_net = cn
   // clear nn->client_tracking_ops

After nfsd mounts, users can trigger grace_done callbacks via
/proc/fs/nfsd/v4_end_grace. If resources are uninitialized or freed
in error paths, this causes access violations.

Resolve the issue by leveraging nfsd_mutex to prevent concurrency.

Fixes: 52e19c09a183 ("nfsd: make reclaim_str_hashtbl allocated per net")
Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
  Changes in v2:
    Use nfsd_mutex instead of adding a new flag to prevent concurrency.
 fs/nfsd/nfs4recover.c | 8 ++++++++
 fs/nfsd/nfs4state.c   | 4 ++++
 fs/nfsd/nfsctl.c      | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 82785db730d9..8ac089f8134c 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -162,7 +162,9 @@ legacy_recdir_name_error(struct nfs4_client *clp, int error)
 	if (error == -ENOENT) {
 		printk(KERN_ERR "NFSD: disabling legacy clientid tracking. "
 			"Reboot recovery will not function correctly!\n");
+		mutex_lock(&nfsd_mutex);
 		nfsd4_client_tracking_exit(clp->net);
+		mutex_unlock(&nfsd_mutex);
 	}
 }
 
@@ -2083,8 +2085,10 @@ nfsd4_client_record_create(struct nfs4_client *clp)
 {
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
+	mutex_lock(&nfsd_mutex);
 	if (nn->client_tracking_ops)
 		nn->client_tracking_ops->create(clp);
+	mutex_unlock(&nfsd_mutex);
 }
 
 void
@@ -2092,8 +2096,10 @@ nfsd4_client_record_remove(struct nfs4_client *clp)
 {
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
+	mutex_lock(&nfsd_mutex);
 	if (nn->client_tracking_ops)
 		nn->client_tracking_ops->remove(clp);
+	mutex_unlock(&nfsd_mutex);
 }
 
 int
@@ -2101,8 +2107,10 @@ nfsd4_client_record_check(struct nfs4_client *clp)
 {
 	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
 
+	mutex_lock(&nfsd_mutex);
 	if (nn->client_tracking_ops)
 		return nn->client_tracking_ops->check(clp);
+	mutex_unlock(&nfsd_mutex);
 
 	return -EOPNOTSUPP;
 }
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d5694987f86f..2794fdc8b678 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2529,7 +2529,9 @@ static void inc_reclaim_complete(struct nfs4_client *clp)
 			nn->reclaim_str_hashtbl_size) {
 		printk(KERN_INFO "NFSD: all clients done reclaiming, ending NFSv4 grace period (net %x)\n",
 				clp->net->ns.inum);
+		mutex_lock(&nfsd_mutex);
 		nfsd4_end_grace(nn);
+		mutex_unlock(&nfsd_mutex);
 	}
 }
 
@@ -6773,7 +6775,9 @@ nfs4_laundromat(struct nfsd_net *nn)
 		lt.new_timeo = 0;
 		goto out;
 	}
+	mutex_lock(&nfsd_mutex);
 	nfsd4_end_grace(nn);
+	mutex_unlock(&nfsd_mutex);
 
 	spin_lock(&nn->s2s_cp_lock);
 	idr_for_each_entry(&nn->s2s_cp_stateids, cps_t, i) {
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 3f3e9f6c4250..649850b4bb60 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1085,7 +1085,9 @@ static ssize_t write_v4_end_grace(struct file *file, char *buf, size_t size)
 			if (!nn->nfsd_serv)
 				return -EBUSY;
 			trace_nfsd_end_grace(netns(file));
+			mutex_lock(&nfsd_mutex);
 			nfsd4_end_grace(nn);
+			mutex_lock(&nfsd_mutex);
 			break;
 		default:
 			return -EINVAL;
-- 
2.46.1


             reply	other threads:[~2025-06-12  4:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-12  3:55 Li Lingfeng [this message]
2025-06-17 20:49 ` [PATCH v2] nfsd: Invoke tracking callbacks only after initialization is complete Jeff Layton
2025-06-18 12:35 ` Jeff Layton
2025-06-18 12:50   ` Jeff Layton
2025-06-18 20:45 ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250612035506.3651985-1-lilingfeng3@huawei.com \
    --to=lilingfeng3@huawei.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=houtao1@huawei.com \
    --cc=jlayton@kernel.org \
    --cc=lilingfeng@huaweicloud.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai1@huaweicloud.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.