Hi,
When traversing the mount list, the current mount point is locked
with vfs_busy(). When a FOREACH_SAFE macro is used, the next pointer
is not locked and could be freed by another process.
I remember that someone showed me a panic that ended in one of these
functions, but I have lost the bug report.
Anyway, the fix is easy in most places, do not use _SAFE as it is
unsafe. In vfs_unmountall() the current pointer is actullay freed.
I have added a comment and will think about it again after my other
dounmount() fix has been commited.
The problem has been introduced in 2013 with this commit message.
> Change 'mountlist' from CIRCLEQ to TAILQ. Be paranoid and
> use TAILQ_*_SAFE more than might be needed.
ok?
bluhm
Index: kern/vfs_subr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.256
diff -u -p -r1.256 vfs_subr.c
--- kern/vfs_subr.c 10 Jan 2017 19:56:34 -0000 1.256
+++ kern/vfs_subr.c 12 Jan 2017 15:52:51 -0000
@@ -1251,12 +1251,12 @@ vprint(char *label, struct vnode *vp)
void
printlockedvnodes(void)
{
- struct mount *mp, *nmp;
+ struct mount *mp;
struct vnode *vp;
printf("Locked vnodes\n");
- TAILQ_FOREACH_SAFE(mp, &mountlist, mnt_list, nmp) {
+ TAILQ_FOREACH(mp, &mountlist, mnt_list) {
if (vfs_busy(mp, VB_READ|VB_NOWAIT))
continue;
LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
@@ -1574,8 +1574,9 @@ vfs_unmountall(void)
retry:
allerror = 0;
TAILQ_FOREACH_REVERSE_SAFE(mp, &mountlist, mntlist, mnt_list, nmp) {
- if ((vfs_busy(mp, VB_WRITE|VB_NOWAIT)) != 0)
+ if (vfs_busy(mp, VB_WRITE|VB_NOWAIT))
continue;
+ /* XXX Here is a race, the next pointer is not locked. */
if ((error = dounmount(mp, MNT_FORCE, curproc)) != 0) {
printf("unmount of %s failed with error %d\n",
mp->mnt_stat.f_mntonname, error);
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.267
diff -u -p -r1.267 vfs_syscalls.c
--- kern/vfs_syscalls.c 10 Jan 2017 20:13:17 -0000 1.267
+++ kern/vfs_syscalls.c 12 Jan 2017 15:44:37 -0000
@@ -425,10 +425,10 @@ struct ctldebug debug0 = { "syncprt", &s
int
sys_sync(struct proc *p, void *v, register_t *retval)
{
- struct mount *mp, *nmp;
+ struct mount *mp;
int asyncflag;
- TAILQ_FOREACH_REVERSE_SAFE(mp, &mountlist, mntlist, mnt_list, nmp) {
+ TAILQ_FOREACH_REVERSE(mp, &mountlist, mntlist, mnt_list) {
if (vfs_busy(mp, VB_READ|VB_NOWAIT))
continue;
if ((mp->mnt_flag & MNT_RDONLY) == 0) {
@@ -570,7 +570,7 @@ sys_getfsstat(struct proc *p, void *v, r
syscallarg(size_t) bufsize;
syscallarg(int) flags;
} */ *uap = v;
- struct mount *mp, *nmp;
+ struct mount *mp;
struct statfs *sp;
struct statfs *sfsp;
size_t count, maxcount;
@@ -580,7 +580,7 @@ sys_getfsstat(struct proc *p, void *v, r
sfsp = SCARG(uap, buf);
count = 0;
- TAILQ_FOREACH_SAFE(mp, &mountlist, mnt_list, nmp) {
+ TAILQ_FOREACH(mp, &mountlist, mnt_list) {
if (vfs_busy(mp, VB_READ|VB_NOWAIT))
continue;
if (sfsp && count < maxcount) {
