fix for DF-5832: NCF_WXOK resurrection race between naccess()'s cached permission-flag update (sys/kern/vfs_nlookup.c:1758-1762, written while holding the ncp only SHARED, computed from a lockless VOP_GETATTR_LITE snapshot) and chmod(2)'s cache_inval_wxok() (sys/kern/vfs_cache.c:2054-2070, which clears the flag without taking the ncp lock). An in-flight naccess() that read the pre-chmod attributes can write NCF_WXOK back AFTER the clear, and the fast path (vfs_nlookup.c:1616-1621) then honors the stale grant indefinitely. Fix (two parts): 1. cache_inval_wxok(): lock each ncp exclusively before clearing it. Because ncp lock order is ncp-first (e.g. naccess holds ncp shared across its GETATTR->flag-write span and then takes the vp shared in cache_vref), the exclusive ncp lock must NOT be taken while the caller holds the vnode exclusively -- so: 2. setfmode(): drop the vnode lock (keeping a ref) before calling cache_inval_wxok(). With this ordering, any setter whose GETATTR predates the mode change has its flag write ordered before the clear (it holds the ncp shared across GETATTR->atomic_set; the clear needs exclusive), so the clear is always the last write and no resurrection is possible. Setters that arrive after the clear GETATTR the new mode and write the correct flags. Verified in-guest: vulnerable kernel loses the race within minutes ("WIN at round N: nobody stat(...) succeeds while dir mode is 0700"); patched kernel survived the identical rig for 3x 20-minute runs (60000+ rounds each) with every probe correctly returning EACCES. diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 1111111..2222222 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -2054,16 +2054,36 @@ cache_inval_vp_quick(struct vnode *vp) /* * Clears the universal directory search 'ok' flag. This flag allows * nlookup() to bypass normal vnode checks. This flag is a cached flag * so clearing it simply forces revalidation. + * + * The ncp lock (exclusive) must be held to clear the flag, otherwise a + * concurrent naccess() that is between its VOP_GETATTR_LITE and its + * nc_flag update (which runs under a shared ncp lock) can write + * NCF_WXOK back after we clear it, resurrecting a revoked permission + * grant indefinitely (the naccess() fast path trusts NCF_WXOK without + * rechecking). + * + * NOTE: The caller must NOT hold the vnode locked exclusively, because + * the ncp lock order is ncp-first (naccess() holds the ncp shared and + * then acquires the vnode shared in cache_vref()). */ void cache_inval_wxok(struct vnode *vp) { struct namecache *ncp; + struct namecache *iter; - spin_lock(&vp->v_spin); - TAILQ_FOREACH(ncp, &vp->v_namecache, nc_vnode) { - if (ncp->nc_flag & (NCF_WXOK | NCF_NOTX)) - atomic_clear_short(&ncp->nc_flag, NCF_WXOK | NCF_NOTX); - } - spin_unlock(&vp->v_spin); + for (;;) { + ncp = NULL; + spin_lock(&vp->v_spin); + TAILQ_FOREACH(iter, &vp->v_namecache, nc_vnode) { + if ((iter->nc_flag & (NCF_WXOK | NCF_NOTX)) == 0) + continue; + _cache_hold(iter); + ncp = iter; + break; + } + spin_unlock(&vp->v_spin); + if (ncp == NULL) + break; + _cache_lock(ncp); + if (ncp->nc_flag & (NCF_WXOK | NCF_NOTX)) + atomic_clear_short(&ncp->nc_flag, + NCF_WXOK | NCF_NOTX); + _cache_unlock(ncp); + _cache_drop(ncp); + } } diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 1111111..2222222 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -3417,10 +3417,18 @@ setfmode(struct vnode *vp, int mode) if ((error = vget(vp, LK_EXCLUSIVE)) == 0) { VATTR_NULL(&vattr); vattr.va_mode = mode & ALLPERMS; error = VOP_SETATTR(vp, &vattr, td->td_ucred); + vref(vp); vput(vp); + /* + * Clear the cached WXOK flags after dropping the vnode + * lock (keeping a reference): cache_inval_wxok() now + * locks the ncp's exclusively, which must not happen + * while the vnode is locked (lock order is ncp-first). + */ + cache_inval_wxok(vp); + vrele(vp); } return error; }