While studying a bug report from naddy@ in 2017 when testing guenther@'s
amap/anon locking diff I figured out that we have been too optimistic in
the !MAP_ANON case.
The reported panic involves, I'd guess, a race between fd_getfile() and
vref():
panic: vref used where vget required
db_enter() at db_enter+0x5
panic() at panic+0x129
vref(ffffff03b20d29e8) at vref+0x5d
uvn_attach(1010000,ffffff03a5879dc0) at uvn_attach+0x11d
uvm_mmapfile(7,ffffff03a5879dc0,2,1,13,100000012) at uvm_mmapfile+0x12c
sys_mmap(c50,ffff8000225f82a0,1) at sys_mmap+0x604
syscall() at syscall+0x279
--- syscall (number 198) ---
end of kernel
Removing the KERNEL_LOCK() from file mapping was out of the scope of this
previous work, so I'd like to go back to a single KERNEL_LOCK/UNLOCK dance
in this code path to remove any false positive.
Note that this code is currently always run under KERNEL_LOCK() so this
will only have effect once the syscall will be unlocked.
ok?
Index: uvm/uvm_mmap.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
retrieving revision 1.161
diff -u -p -r1.161 uvm_mmap.c
--- uvm/uvm_mmap.c 4 Mar 2020 21:15:39 -0000 1.161
+++ uvm/uvm_mmap.c 28 Sep 2020 09:48:26 -0000
@@ -288,8 +288,11 @@ sys_mmap(struct proc *p, void *v, regist
/* check for file mappings (i.e. not anonymous) and verify file. */
if ((flags & MAP_ANON) == 0) {
- if ((fp = fd_getfile(fdp, fd)) == NULL)
- return (EBADF);
+ KERNEL_LOCK();
+ if ((fp = fd_getfile(fdp, fd)) == NULL) {
+ error = EBADF;
+ goto out;
+ }
if (fp->f_type != DTYPE_VNODE) {
error = ENODEV; /* only mmap vnodes! */
@@ -313,6 +316,7 @@ sys_mmap(struct proc *p, void *v, regist
flags |= MAP_ANON;
FRELE(fp, p);
fp = NULL;
+ KERNEL_UNLOCK();
goto is_anon;
}
@@ -362,9 +366,7 @@ sys_mmap(struct proc *p, void *v, regist
* EPERM.
*/
if (fp->f_flag & FWRITE) {
- KERNEL_LOCK();
error = VOP_GETATTR(vp, &va, p->p_ucred, p);
- KERNEL_UNLOCK();
if (error)
goto out;
if ((va.va_flags & (IMMUTABLE|APPEND)) == 0)
@@ -390,9 +392,9 @@ sys_mmap(struct proc *p, void *v, regist
goto out;
}
}
- KERNEL_LOCK();
error = uvm_mmapfile(&p->p_vmspace->vm_map, &addr, size, prot,
maxprot, flags, vp, pos, lim_cur(RLIMIT_MEMLOCK), p);
+ FRELE(fp, p);
KERNEL_UNLOCK();
} else { /* MAP_ANON case */
if (fd != -1)
@@ -428,7 +430,10 @@ is_anon: /* label for SunOS style /dev/z
/* remember to add offset */
*retval = (register_t)(addr + pageoff);
+ return (error);
+
out:
+ KERNEL_UNLOCK();
if (fp)
FRELE(fp, p);
return (error);
