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@xxxxxxxxxx> --- 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