The following reply was made to PR kern/53096; it has been noted by GNATS.
From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc:
Subject: Re: kern/53096 (netbsd-8 crash on heavy disk I/O)
Date: Fri, 6 Apr 2018 12:15:22 +0200
From a recent crash it looks like a race condition from procfs_dir().
One proc is running stat or readlink on /proc/XXX/cwd while proc XXX
changes its working directory. As procfs_dir() takes the cwdi_cdir
without locking the cwdi it may be vrele'd before getcwd_common()
adds a reference resulting in the panics observed for this PR.
Please apply this patch, it should fix the race.
diff -r eeeea698d964 -r 18ca5463ad55 sys/miscfs/procfs/procfs_vnops.c
--- sys/miscfs/procfs/procfs_vnops.c
+++ sys/miscfs/procfs/procfs_vnops.c
@@ -545,11 +545,10 @@ procfs_symlink(void *v)
}
/*
- * Works out the path to (and vnode of) the target process's current
+ * Works out the path to the target process's current
* working directory or chroot. If the caller is in a chroot and
* can't "reach" the target's cwd or root (or some other error
- * occurs), a "/" is returned for the path and a NULL pointer is
- * returned for the vnode.
+ * occurs), a "/" is returned for the path.
*/
static void
procfs_dir(pfstype t, struct lwp *caller, struct proc *target, char **bpp,
@@ -559,12 +558,12 @@ procfs_dir(pfstype t, struct lwp *caller
struct vnode *vp, *rvp;
char *bp;
- cwdi = caller->l_proc->p_cwdi;
- rw_enter(&cwdi->cwdi_lock, RW_READER);
-
- rvp = cwdi->cwdi_rdir;
- bp = bpp ? *bpp : NULL;
-
+ /*
+ * Lock target cwdi and take a reference to the vnode
+ * we are interested in to prevent it from disappearing
+ * before getcwd_common() below.
+ */
+ rw_enter(&target->p_cwdi->cwdi_lock, RW_READER);
switch (t) {
case PFScwd:
vp = target->p_cwdi->cwdi_cdir;
@@ -573,9 +572,17 @@ procfs_dir(pfstype t, struct lwp *caller
vp = target->p_cwdi->cwdi_rdir;
break;
default:
- rw_exit(&cwdi->cwdi_lock);
+ rw_exit(&target->p_cwdi->cwdi_lock);
return;
}
+ vref(vp);
+ rw_exit(&target->p_cwdi->cwdi_lock);
+
+ cwdi = caller->l_proc->p_cwdi;
+ rw_enter(&cwdi->cwdi_lock, RW_READER);
+
+ rvp = cwdi->cwdi_rdir;
+ bp = bpp ? *bpp : NULL;
/*
* XXX: this horrible kludge avoids locking panics when
@@ -586,6 +593,7 @@ procfs_dir(pfstype t, struct lwp *caller
*--bp = '/';
*bpp = bp;
}
+ vrele(vp);
rw_exit(&cwdi->cwdi_lock);
return;
}
@@ -594,7 +602,6 @@ procfs_dir(pfstype t, struct lwp *caller
rvp = rootvnode;
if (vp == NULL || getcwd_common(vp, rvp, bp ? &bp : NULL, path,
len / 2, 0, caller) != 0) {
- vp = NULL;
if (bpp) {
bp = *bpp;
*--bp = '/';
@@ -604,6 +611,8 @@ procfs_dir(pfstype t, struct lwp *caller
if (bpp)
*bpp = bp;
+ if (vp != NULL)
+ vrele(vp);
rw_exit(&cwdi->cwdi_lock);
}