Date: Sat, 04 Jul 2026 22:01:30 +0000 From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: fe6677e7f440 - main - nfs_clstate.c: Fix handling of delegation upgrades Message-ID: <6a4982ba.1dfc8.46aecc98@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=fe6677e7f440d1aa52de036639efc55047ab9a2b commit fe6677e7f440d1aa52de036639efc55047ab9a2b Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2026-07-04 22:00:02 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2026-07-04 22:00:02 +0000 nfs_clstate.c: Fix handling of delegation upgrades Commit 016570c4463d modified the client to handle the upgrade of a read delegation to a write delegation, where the server provides the same delegation stateid to the client. However, it failed to check if the delegation structure was currently in use. Without this patch, if the structure was in use, a use after free could occur. This patch handles the "in use" case by copying the necessary fields into the current/old structure and free's the new one instead of the old one that is "in use". PR: 296224 MFC after: 2 weeks --- sys/fs/nfs/nfsclstate.h | 6 ++++-- sys/fs/nfsclient/nfs_clstate.c | 45 +++++++++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/sys/fs/nfs/nfsclstate.h b/sys/fs/nfs/nfsclstate.h index 860a2bd1d4c3..ce8fc7018c4b 100644 --- a/sys/fs/nfs/nfsclstate.h +++ b/sys/fs/nfs/nfsclstate.h @@ -162,17 +162,19 @@ struct nfscldeleg { struct nfsclownerhead nfsdl_owner; /* locally issued state */ struct nfscllockownerhead nfsdl_lock; nfsv4stateid_t nfsdl_stateid; - struct acl_entry nfsdl_ace; /* Delegation ace */ struct nfsclclient *nfsdl_clp; struct nfsv4lock nfsdl_rwlock; /* for active I/O ops */ +#define nfsdl_startcopy nfsdl_ace + struct acl_entry nfsdl_ace; /* Delegation ace */ struct nfscred nfsdl_cred; /* Cred. used for Open */ time_t nfsdl_timestamp; /* used for stale cleanup */ u_int64_t nfsdl_sizelimit; /* Limit for file growth */ u_int64_t nfsdl_size; /* saved copy of file size */ u_int64_t nfsdl_change; /* and change attribute */ struct timespec nfsdl_modtime; /* local modify time */ - u_int16_t nfsdl_fhlen; u_int8_t nfsdl_flags; +#define nfsdl_endcopy nfsdl_fhlen + u_int16_t nfsdl_fhlen; u_int8_t nfsdl_fh[1]; /* must be last */ }; diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index e906de603c28..aa3aef876cbe 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -467,8 +467,11 @@ nfscl_deleg(mount_t mp, struct nfsclclient *clp, u_int8_t *nfhp, } else { /* * A delegation already exists. If the new one is a Write - * delegation and the old one a Read delegation, return the - * Read delegation. Otherwise, return the new delegation. + * delegation and the old one a Read delegation, upgrade + * if possible. The upgrade can be done if either: + * - The stateid is the same (true for the FreeBSD nfs server). + * - The current delegation is not locked. + * Otherwise, return the new delegation. */ if (dp != NULL) { if (NFSBCMP(dp->nfsdl_stateid.other, @@ -476,13 +479,37 @@ nfscl_deleg(mount_t mp, struct nfsclclient *clp, u_int8_t *nfhp, trydelegret = true; if ((dp->nfsdl_flags & NFSCLDL_WRITE) != 0 && (tdp->nfsdl_flags & NFSCLDL_READ) != 0) { - TAILQ_REMOVE(&clp->nfsc_deleg, tdp, nfsdl_list); - LIST_REMOVE(tdp, nfsdl_hash); - TAILQ_INSERT_HEAD(&clp->nfsc_deleg, dp, - nfsdl_list); - LIST_INSERT_HEAD(NFSCLDELEGHASH(clp, nfhp, - fhlen), dp, nfsdl_hash); - dp->nfsdl_timestamp = NFSD_MONOSEC + 120; + if (tdp->nfsdl_rwlock.nfslock_usecnt == 0 && + tdp->nfsdl_rwlock.nfslock_lock == 0) { + TAILQ_REMOVE(&clp->nfsc_deleg, tdp, + nfsdl_list); + LIST_REMOVE(tdp, nfsdl_hash); + TAILQ_INSERT_HEAD(&clp->nfsc_deleg, dp, + nfsdl_list); + LIST_INSERT_HEAD(NFSCLDELEGHASH(clp, + nfhp, fhlen), dp, nfsdl_hash); + dp->nfsdl_timestamp = NFSD_MONOSEC + + 120; + } else if (!trydelegret) { + /* + * For this case, the fields in + * struct nfscldeleg that change for a + * write delegation must be copied. + * This way, the delegation structure + * does not get free'd prematurely. + */ + memcpy(&tdp->nfsdl_startcopy, + &dp->nfsdl_startcopy, + __rangeof(struct nfscldeleg, + nfsdl_startcopy, nfsdl_endcopy)); + TAILQ_REMOVE(&clp->nfsc_deleg, tdp, + nfsdl_list); + TAILQ_INSERT_HEAD(&clp->nfsc_deleg, tdp, + nfsdl_list); + tdp->nfsdl_timestamp = NFSD_MONOSEC + + 120; + tdp = dp; /* Free the new one. */ + } } else { tdp = dp; /* Return this one. */ }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a4982ba.1dfc8.46aecc98>