DF-2918 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 | --- a/sys/kern/vfs_init.c 2026-09-03 10:26:07.445442997 +0000 +++ b/sys/kern/vfs_init.c 2026-09-03 10:52:21.885651979 +0000 @@ -78,6 +78,7 @@ #include <sys/vnode.h> #include <sys/malloc.h> #include <sys/objcache.h> +#include <sys/lock.h> static MALLOC_DEFINE(M_VNODEOP, "vnodeops", "vnode operations vectors"); static MALLOC_DEFINE(M_NAMEI, "nameibufs", "namei path buffers"); @@ -90,6 +91,41 @@ static TAILQ_HEAD(, vnodeopv_node) vnodeopv_list; static void vfs_calc_vnodeops(struct vop_ops *ops); +/* + * Registry lock. Held shared by lookups, iteration and the atomic + * lookup+refcount acquisition in vfsconf_acquire(); held exclusive by + * registration and by vfs_unregister's refcount-check/remove critical + * section. This closes the TOCTOU where a kldunload could observe a + * zero vfc_refcount while a concurrent sys_mount() was between its + * vfsconf_find_by_name() and vfc_refcount++, and unregisters/frees the + * vfsconf (module .data) out from under the mounting thread. + */ +static struct lock vfsconf_lk; + +/* + * Number of mount(2) syscalls currently in flight. vfs_unregister() + * refuses to tear a filesystem module down while any mount syscall is + * running: sys_mount() touches module memory (vfsconf/vfsops) from + * before its vfsconf_acquire() until after VFS_MOUNT(), and its + * auto-load path calls into the linker while holding the mountpoint + * vnode lock — which can deadlock against kldunload holding the linker + * lock through module teardown. This veto removes the whole class of + * mount-vs-module-unload interleavings. + */ +static volatile int vfsconf_mounts_inflight; + +void +vfsconf_mount_begin(void) +{ + atomic_add_int(&vfsconf_mounts_inflight, 1); +} + +void +vfsconf_mount_end(void) +{ + atomic_add_int(&vfsconf_mounts_inflight, -1); +} + /* * Add a vnode operations (vnops) vector to the global list. @@ -204,6 +240,7 @@ vfsinit(void *dummy) { TAILQ_INIT(&vnodeopv_list); + lockinit(&vfsconf_lk, "vfscflk", 0, 0); namei_oc = objcache_create_simple(M_NAMEI, MAXPATHLEN); /* @@ -237,8 +274,9 @@ static STAILQ_HEAD(, vfsconf) vfsconf_list = STAILQ_HEAD_INITIALIZER(vfsconf_list); -struct vfsconf * -vfsconf_find_by_name(const char *name) +/* caller must hold vfsconf_lk (shared or exclusive) */ +static struct vfsconf * +vfsconf_find_by_name_locked(const char *name) { struct vfsconf *vfsp; @@ -250,7 +288,19 @@ } struct vfsconf * -vfsconf_find_by_typenum(int typenum) +vfsconf_find_by_name(const char *name) +{ + struct vfsconf *vfsp; + + lockmgr(&vfsconf_lk, LK_SHARED); + vfsp = vfsconf_find_by_name_locked(name); + lockmgr(&vfsconf_lk, LK_RELEASE); + return vfsp; +} + +/* caller must hold vfsconf_lk (shared or exclusive) */ +static struct vfsconf * +vfsconf_find_by_typenum_locked(int typenum) { struct vfsconf *vfsp; @@ -261,6 +311,44 @@ return vfsp; } +struct vfsconf * +vfsconf_find_by_typenum(int typenum) +{ + struct vfsconf *vfsp; + + lockmgr(&vfsconf_lk, LK_SHARED); + vfsp = vfsconf_find_by_typenum_locked(typenum); + lockmgr(&vfsconf_lk, LK_RELEASE); + return vfsp; +} + +/* + * Acquire a counted reference on a filesystem type by name. The + * refcount increment is atomic with the list lookup (the shared + * registry lock excludes vfs_unregister()'s exclusive check-and-remove + * section), so a vfsconf returned here cannot be unregistered/freed + * while the reference is held. Pair with vfsconf_release(). + */ +struct vfsconf * +vfsconf_acquire(const char *name) +{ + struct vfsconf *vfsp; + + lockmgr(&vfsconf_lk, LK_SHARED); + vfsp = vfsconf_find_by_name_locked(name); + if (vfsp != NULL) + atomic_add_int(&vfsp->vfc_refcount, 1); + lockmgr(&vfsconf_lk, LK_RELEASE); + return vfsp; +} + +void +vfsconf_release(struct vfsconf *vfc) +{ + atomic_add_int(&vfc->vfc_refcount, -1); +} + +/* caller must hold vfsconf_lk exclusively */ static void vfsconf_add(struct vfsconf *vfc) { @@ -268,6 +356,7 @@ STAILQ_INSERT_TAIL(&vfsconf_list, vfc, vfc_next); } +/* caller must hold vfsconf_lk exclusively */ static void vfsconf_remove(struct vfsconf *vfc) { @@ -292,6 +381,8 @@ /* * Iterate over all vfsconf entries. Break out of the iterator * by returning != 0. + * + * The registry lock is held shared across the iteration. */ int vfsconf_each(int (*iter)(struct vfsconf *element, void *data), void *data) @@ -299,12 +390,15 @@ int error; struct vfsconf *vfsp; + error = 0; + lockmgr(&vfsconf_lk, LK_SHARED); STAILQ_FOREACH(vfsp, &vfsconf_list, vfc_next) { error = iter(vfsp, data); if (error) - return (error); + break; } - return (0); + lockmgr(&vfsconf_lk, LK_RELEASE); + return (error); } /* @@ -320,10 +414,14 @@ struct sysctl_oid *oidp; struct vfsops *vfsops = NULL; - if (vfsconf_find_by_name(vfc->vfc_name) != NULL) + lockmgr(&vfsconf_lk, LK_EXCLUSIVE); + if (vfsconf_find_by_name_locked(vfc->vfc_name) != NULL) { + lockmgr(&vfsconf_lk, LK_RELEASE); return EEXIST; + } vfsconf_add(vfc); + lockmgr(&vfsconf_lk, LK_RELEASE); /* * If this filesystem has a sysctl node under vfs @@ -461,21 +559,44 @@ struct vfsconf *vfsp; int error; - vfsp = vfsconf_find_by_name(vfc->vfc_name); + /* + * The refcount check and the removal are one exclusive critical + * section, excluding vfsconf_acquire() (shared), so a mount can + * never race a reference onto a vfsconf that is being torn down. + */ + lockmgr(&vfsconf_lk, LK_EXCLUSIVE); + vfsp = vfsconf_find_by_name_locked(vfc->vfc_name); - if (vfsp == NULL) + if (vfsp == NULL) { + lockmgr(&vfsconf_lk, LK_RELEASE); return EINVAL; + } + + if (vfsp->vfc_refcount != 0) { + lockmgr(&vfsconf_lk, LK_RELEASE); + return EBUSY; + } - if (vfsp->vfc_refcount != 0) + /* + * Refuse to race any in-flight mount(2): mounts touch the module's + * vfsconf/vfsops (and the linker, for auto-loaded types) from + * before the refcount acquisition until after VFS_MOUNT(). + */ + if (vfsconf_mounts_inflight != 0) { + lockmgr(&vfsconf_lk, LK_RELEASE); return EBUSY; + } if (vfc->vfc_vfsops->vfs_uninit != NULL) { error = vfs_uninit(vfc, vfsp); - if (error) + if (error) { + lockmgr(&vfsconf_lk, LK_RELEASE); return (error); + } } vfsconf_remove(vfsp); + lockmgr(&vfsconf_lk, LK_RELEASE); return 0; } --- a/sys/kern/vfs_mount.c 2026-09-03 10:26:07.445442997 +0000 +++ b/sys/kern/vfs_mount.c 2026-09-03 10:27:06.928684162 +0000 @@ -337,7 +337,7 @@ if (fstypename == NULL) return (ENODEV); - vfsp = vfsconf_find_by_name(fstypename); + vfsp = vfsconf_acquire(fstypename); if (vfsp == NULL) return (ENODEV); mp = kmalloc(sizeof(struct mount), M_MOUNT, M_WAITOK | M_ZERO); @@ -348,7 +348,6 @@ vfs_busy(mp, 0); mp->mnt_vfc = vfsp; mp->mnt_pbuf_count = nswbuf_kva / NSWBUF_SPLIT; - vfsp->vfc_refcount++; mp->mnt_stat.f_type = vfsp->vfc_typenum; mp->mnt_flag |= MNT_RDONLY; mp->mnt_flag |= vfsp->vfc_flags & MNT_VISFLAGMASK; --- a/sys/kern/vfs_syscalls.c 2026-09-03 10:26:07.445442997 +0000 +++ b/sys/kern/vfs_syscalls.c 2026-09-03 10:52:54.205243728 +0000 @@ -133,6 +133,14 @@ cred = td->td_ucred; + /* + * Mark the mount syscall in-flight so vfs module teardown + * (vfs_unregister via kldunload) vetoes with EBUSY instead of + * racing us for the vfsconf/vfsops lifetime or deadlocking + * against the auto-load path below. + */ + vfsconf_mount_begin(); + /* We do not allow user mounts inside a jail for now */ if (usermount && jailed(cred)) { error = EPERM; @@ -310,7 +318,7 @@ error = EPERM; goto done; } - vfsp = vfsconf_find_by_name(fstypename); + vfsp = vfsconf_acquire(fstypename); if (vfsp == NULL) { linker_file_t lf; @@ -331,7 +339,7 @@ } lf->userrefs++; /* lookup again, see if the VFS was loaded */ - vfsp = vfsconf_find_by_name(fstypename); + vfsp = vfsconf_acquire(fstypename); if (vfsp == NULL) { lf->userrefs--; linker_file_unload(lf); @@ -356,7 +364,6 @@ vfs_busy(mp, LK_NOWAIT); mp->mnt_vfc = vfsp; mp->mnt_pbuf_count = nswbuf_kva / NSWBUF_SPLIT; - vfsp->vfc_refcount++; mp->mnt_stat.f_type = vfsp->vfc_typenum; mp->mnt_flag |= vfsp->vfc_flags & MNT_VISFLAGMASK; strncpy(mp->mnt_stat.f_fstypename, vfsp->vfc_name, MFSNAMELEN); @@ -465,7 +472,7 @@ crfree(mp->mnt_cred); mp->mnt_cred = NULL; } - mp->mnt_vfc->vfc_refcount--; + vfsconf_release(mp->mnt_vfc); lwkt_reltoken(&mp->mnt_token); vfs_unbusy(mp); kfree(mp, M_MOUNT); @@ -473,6 +480,7 @@ vput(vp); } done: + vfsconf_mount_end(); return (error); } @@ -1076,7 +1084,7 @@ mp->mnt_cred = NULL; } - mp->mnt_vfc->vfc_refcount--; + vfsconf_release(mp->mnt_vfc); /* * If not quickhalting the mount, we expect there to be no --- a/sys/sys/mount.h 2026-09-03 10:26:07.449442946 +0000 +++ b/sys/sys/mount.h 2026-09-03 10:52:43.601377687 +0000 @@ -532,6 +532,12 @@ struct vfsconf *vfsconf_find_by_name(const char *); struct vfsconf *vfsconf_find_by_typenum(int); int vfsconf_get_maxtypenum(void); + +struct vfsconf *vfsconf_acquire(const char *); +void vfsconf_release(struct vfsconf *); +void vfsconf_mount_begin(void); +void vfsconf_mount_end(void); + int vfsconf_each(int (*)(struct vfsconf *, void *), void *); #endif |