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. Instead of adding locks for specific resources(e.g., reclaim_str_hashtbl), introducing a flag to indicate whether tracking initialization has completed and checking this flag before invoking callbacks may be better. Fixes: 52e19c09a183 ("nfsd: make reclaim_str_hashtbl allocated per net") Signed-off-by: Li Lingfeng <lilingfeng3@xxxxxxxxxx> --- fs/nfsd/netns.h | 1 + fs/nfsd/nfs4recover.c | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index 3e2d0fde80a7..dbd782d6b063 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -113,6 +113,7 @@ struct nfsd_net { struct file *rec_file; bool in_grace; + bool client_tracking_init_done; const struct nfsd4_client_tracking_ops *client_tracking_ops; time64_t nfsd4_lease; diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index c1d9bd07285f..6c27c1252c0e 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -2096,7 +2096,11 @@ nfsd4_client_tracking_init(struct net *net) pr_warn("NFSD: Unable to initialize client recovery tracking! (%d)\n", status); pr_warn("NFSD: Is nfsdcld running? If not, enable CONFIG_NFSD_LEGACY_CLIENT_TRACKING.\n"); nn->client_tracking_ops = NULL; + nn->client_tracking_init_done = false; + } else { + nn->client_tracking_init_done = true; } + return status; } @@ -2105,6 +2109,7 @@ nfsd4_client_tracking_exit(struct net *net) { struct nfsd_net *nn = net_generic(net, nfsd_net_id); + nn->client_tracking_init_done = false; if (nn->client_tracking_ops) { if (nn->client_tracking_ops->exit) nn->client_tracking_ops->exit(net); @@ -2117,7 +2122,7 @@ nfsd4_client_record_create(struct nfs4_client *clp) { struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); - if (nn->client_tracking_ops) + if (nn->client_tracking_ops && nn->client_tracking_init_done) nn->client_tracking_ops->create(clp); } @@ -2126,7 +2131,7 @@ nfsd4_client_record_remove(struct nfs4_client *clp) { struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); - if (nn->client_tracking_ops) + if (nn->client_tracking_ops && nn->client_tracking_init_done) nn->client_tracking_ops->remove(clp); } @@ -2135,7 +2140,7 @@ nfsd4_client_record_check(struct nfs4_client *clp) { struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id); - if (nn->client_tracking_ops) + if (nn->client_tracking_ops && nn->client_tracking_init_done) return nn->client_tracking_ops->check(clp); return -EOPNOTSUPP; @@ -2144,7 +2149,7 @@ nfsd4_client_record_check(struct nfs4_client *clp) void nfsd4_record_grace_done(struct nfsd_net *nn) { - if (nn->client_tracking_ops) + if (nn->client_tracking_ops && nn->client_tracking_init_done) nn->client_tracking_ops->grace_done(nn); } -- 2.31.1