From: Christoph Hellwig Date: Wed, 7 Jan 2026 07:27:08 +0000 (+0100) Subject: NFS: take a delegation reference in nfs4_get_valid_delegation X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=542b11c0728335a1e06f61dc71b48e9bbbe13169;p=thirdparty%2Fkernel%2Flinux.git NFS: take a delegation reference in nfs4_get_valid_delegation Currently most work on struct nfs_delegation happens directly under RCU protection. This is generally fine, despite that long RCU sections are not good for performance. But for operations later taking a reference to the delegation to perform blocking work, refcount_inc is used, which can be racy against dropping the last reference and thus lead to use after frees in extremely rare cases. Fix this by taking a reference in nfs4_get_valid_delegation using refcount_inc_not_zero so that the callers have a stabilized reference they can work with and can be moved outside the RCU critical section. Signed-off-by: Christoph Hellwig Signed-off-by: Anna Schumaker --- diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c index 8397c43358bd1..57550020c8195 100644 --- a/fs/nfs/callback_proc.c +++ b/fs/nfs/callback_proc.c @@ -51,12 +51,18 @@ __be32 nfs4_callback_getattr(void *argp, void *resp, -ntohl(res->status)); goto out; } - rcu_read_lock(); + delegation = nfs4_get_valid_delegation(inode); - if (delegation == NULL || (delegation->type & FMODE_WRITE) == 0) + if (!delegation) goto out_iput; - res->size = i_size_read(inode); + if ((delegation->type & FMODE_WRITE) == 0) { + nfs_put_delegation(delegation); + goto out_iput; + } res->change_attr = delegation->change_attr; + nfs_put_delegation(delegation); + + res->size = i_size_read(inode); if (nfs_have_writebacks(inode)) res->change_attr++; res->atime = inode_get_atime(inode); @@ -71,7 +77,6 @@ __be32 nfs4_callback_getattr(void *argp, void *resp, FATTR4_WORD2_TIME_DELEG_MODIFY) & args->bitmap[2]; res->status = 0; out_iput: - rcu_read_unlock(); trace_nfs4_cb_getattr(cps->clp, &args->fh, inode, -ntohl(res->status)); nfs_iput_and_deactive(inode); out: diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index a64c30efa1a3e..0e91ee9aaa1b6 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -66,7 +66,7 @@ static struct nfs_delegation *nfs_get_delegation(struct nfs_delegation *delegati return delegation; } -static void nfs_put_delegation(struct nfs_delegation *delegation) +void nfs_put_delegation(struct nfs_delegation *delegation) { if (refcount_dec_and_test(&delegation->refcount)) __nfs_free_delegation(delegation); @@ -104,10 +104,14 @@ struct nfs_delegation *nfs4_get_valid_delegation(const struct inode *inode) { struct nfs_delegation *delegation; + rcu_read_lock(); delegation = rcu_dereference(NFS_I(inode)->delegation); - if (nfs4_is_valid_delegation(delegation, 0)) - return delegation; - return NULL; + if (!nfs4_is_valid_delegation(delegation, 0) || + !refcount_inc_not_zero(&delegation->refcount)) + delegation = NULL; + rcu_read_unlock(); + + return delegation; } static int nfs4_do_check_delegation(struct inode *inode, fmode_t type, @@ -794,10 +798,11 @@ void nfs4_inode_set_return_delegation_on_close(struct inode *inode) if (!inode) return; - rcu_read_lock(); + delegation = nfs4_get_valid_delegation(inode); if (!delegation) - goto out; + return; + spin_lock(&delegation->lock); if (!delegation->inode) goto out_unlock; @@ -811,8 +816,7 @@ out_unlock: spin_unlock(&delegation->lock); if (ret) nfs_clear_verifier_delegated(inode); -out: - rcu_read_unlock(); + nfs_put_delegation(delegation); nfs_end_delegation_return(inode, ret, 0); } @@ -828,10 +832,10 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode) struct nfs_delegation *delegation; struct nfs_delegation *ret = NULL; - rcu_read_lock(); delegation = nfs4_get_valid_delegation(inode); if (!delegation) - goto out; + return; + if (test_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags) || atomic_long_read(&NFS_SERVER(inode)->nr_active_delegations) >= nfs_delegation_watermark) { @@ -847,8 +851,8 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode) if (ret) nfs_clear_verifier_delegated(inode); } -out: - rcu_read_unlock(); + + nfs_put_delegation(delegation); nfs_end_delegation_return(inode, ret, 0); } @@ -863,17 +867,17 @@ out: int nfs4_inode_make_writeable(struct inode *inode) { struct nfs_delegation *delegation; + int error = 0; - rcu_read_lock(); delegation = nfs4_get_valid_delegation(inode); - if (delegation == NULL || - (nfs4_has_session(NFS_SERVER(inode)->nfs_client) && - (delegation->type & FMODE_WRITE))) { - rcu_read_unlock(); + if (!delegation) return 0; - } - rcu_read_unlock(); - return nfs4_inode_return_delegation(inode); + + if (!nfs4_has_session(NFS_SERVER(inode)->nfs_client) || + !(delegation->type & FMODE_WRITE)) + error = nfs4_inode_return_delegation(inode); + nfs_put_delegation(delegation); + return error; } static void @@ -1116,24 +1120,24 @@ int nfs_async_inode_return_delegation(struct inode *inode, struct nfs_client *clp = server->nfs_client; struct nfs_delegation *delegation; - rcu_read_lock(); delegation = nfs4_get_valid_delegation(inode); - if (delegation == NULL) - goto out_enoent; + if (!delegation) + return -ENOENT; + if (stateid != NULL && - !clp->cl_mvops->match_stateid(&delegation->stateid, stateid)) - goto out_enoent; + !clp->cl_mvops->match_stateid(&delegation->stateid, stateid)) { + nfs_put_delegation(delegation); + return -ENOENT; + } + nfs_mark_return_delegation(server, delegation); - rcu_read_unlock(); + nfs_put_delegation(delegation); /* If there are any application leases or delegations, recall them */ break_lease(inode, O_WRONLY | O_RDWR | O_NONBLOCK); nfs_delegation_run_state_manager(clp); return 0; -out_enoent: - rcu_read_unlock(); - return -ENOENT; } static struct inode * diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h index fef1f4126e8f3..d1c5da3e66ea1 100644 --- a/fs/nfs/delegation.h +++ b/fs/nfs/delegation.h @@ -80,6 +80,7 @@ bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_state bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode); struct nfs_delegation *nfs4_get_valid_delegation(const struct inode *inode); +void nfs_put_delegation(struct nfs_delegation *delegation); void nfs_mark_delegation_referenced(struct nfs_delegation *delegation); int nfs4_have_delegation(struct inode *inode, fmode_t type, int flags); int nfs4_check_delegation(struct inode *inode, fmode_t type); diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 935ec446e52e6..9d43016d9aa14 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1615,10 +1615,11 @@ static bool can_open_delegated(const struct inode *inode, fmode_t fmode, struct nfs_delegation *delegation; bool ret = false; - rcu_read_lock(); delegation = nfs4_get_valid_delegation(inode); - if (!delegation || (delegation->type & fmode) != fmode) - goto out_unlock; + if (!delegation) + return false; + if ((delegation->type & fmode) != fmode) + goto out_put_delegation; switch (claim) { case NFS4_OPEN_CLAIM_PREVIOUS: @@ -1637,8 +1638,8 @@ static bool can_open_delegated(const struct inode *inode, fmode_t fmode, break; } -out_unlock: - rcu_read_unlock(); +out_put_delegation: + nfs_put_delegation(delegation); return ret; } @@ -1913,10 +1914,11 @@ int update_open_stateid(struct nfs4_state *state, fmode &= (FMODE_READ|FMODE_WRITE); - rcu_read_lock(); spin_lock(&state->owner->so_lock); if (open_stateid != NULL) { + rcu_read_lock(); nfs_state_set_open_stateid(state, open_stateid, fmode, &freeme); + rcu_read_unlock(); ret = 1; } @@ -1940,11 +1942,11 @@ int update_open_stateid(struct nfs4_state *state, ret = 1; no_delegation_unlock: spin_unlock(&deleg_cur->lock); + nfs_put_delegation(deleg_cur); no_delegation: if (ret) update_open_stateflags(state, fmode); spin_unlock(&state->owner->so_lock); - rcu_read_unlock(); if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags)) nfs4_schedule_state_manager(clp); @@ -1978,14 +1980,12 @@ static void nfs4_return_incompatible_delegation(struct inode *inode, fmode_t fmo struct nfs_delegation *delegation; fmode &= FMODE_READ|FMODE_WRITE; - rcu_read_lock(); delegation = nfs4_get_valid_delegation(inode); - if (delegation == NULL || (delegation->type & fmode) == fmode) { - rcu_read_unlock(); + if (!delegation) return; - } - rcu_read_unlock(); - nfs4_inode_return_delegation(inode); + if ((delegation->type & fmode) != fmode) + nfs4_inode_return_delegation(inode); + nfs_put_delegation(delegation); } static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)