vfs lookup crash during unmount

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

vfs lookup crash during unmount

Alexander Bluhm
Hi,

When doing a forced unmount on a stressed file system, it may crash
here.

uvm_fault(0xfffffd85827449a0, 0x50, 0, 1) -> e
kernel: page fault trap, code=0
Stopped at      vfs_lookup+0x5d8:       testb   $0x1,0x50(%rcx)
ddb{2}> bt
vfs_lookup(ffff8000229b22a0) at vfs_lookup+0x5d8
namei(ffff8000229b22a0) at namei+0x395
dounlinkat(ffff8000fffeb1f8,ffffff9c,11339e48878,0) at dounlinkat+0x76
syscall(ffff8000229b2490) at syscall+0x389
Xsyscall(6,a,110cd90a000,a,0,113c729cb80) at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7ffffded50, count: -5

                if (rdonly || (dp->v_mount->mnt_flag & MNT_RDONLY) ||
                    (wantparent &&
                    (ndp->ni_dvp->v_mount->mnt_flag & MNT_RDONLY))) {
                        error = EROFS;
                        goto bad2;
                }

dp->v_mount can be NULL.  I think this can happen at multiple places
in vfs_lookup() when it is sleeping.  Recently vn_lock() got even
more calls to tsleep(9).

We can add checks like this.  It this the right approach?  Or should
we make sure that all vnodes in vfs_lookup() are always locked and
dounmount() has to wait until lock is released?  The latter seems
quite complicated.

Syzcaller creates a crash at the same location, but I think it is
not using unmount.  Don't know if it is related.
https://syzkaller.appspot.com/bug?id=49b54eec7f3e2afc637cf49adf31d04dcca8b69e

bluhm

Index: kern/vfs_lookup.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_lookup.c,v
retrieving revision 1.82
diff -u -p -r1.82 vfs_lookup.c
--- kern/vfs_lookup.c 29 Jul 2019 12:35:19 -0000 1.82
+++ kern/vfs_lookup.c 3 Sep 2019 19:16:15 -0000
@@ -652,6 +652,12 @@ dirloop:
  ndp->ni_vp = dp = tdp;
  }

+ /* In case we were sleeping, file system may be unmounted. */
+ if (dp->v_type == VBAD) {
+ error = ENOENT;
+ goto bad2;
+ }
+
  /*
  * Check for symbolic link.  Back up over any slashes that we skipped,
  * as we will need them again.

Reply | Threaded
Open this post in threaded view
|

Re: vfs lookup crash during unmount

Martin Pieuchot
On 05/09/19(Thu) 23:54, Alexander Bluhm wrote:

> Hi,
>
> When doing a forced unmount on a stressed file system, it may crash
> here.
>
> uvm_fault(0xfffffd85827449a0, 0x50, 0, 1) -> e
> kernel: page fault trap, code=0
> Stopped at      vfs_lookup+0x5d8:       testb   $0x1,0x50(%rcx)
> ddb{2}> bt
> vfs_lookup(ffff8000229b22a0) at vfs_lookup+0x5d8
> namei(ffff8000229b22a0) at namei+0x395
> dounlinkat(ffff8000fffeb1f8,ffffff9c,11339e48878,0) at dounlinkat+0x76
> syscall(ffff8000229b2490) at syscall+0x389
> Xsyscall(6,a,110cd90a000,a,0,113c729cb80) at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7f7ffffded50, count: -5
>
>                 if (rdonly || (dp->v_mount->mnt_flag & MNT_RDONLY) ||
>                     (wantparent &&
>                     (ndp->ni_dvp->v_mount->mnt_flag & MNT_RDONLY))) {
>                         error = EROFS;
>                         goto bad2;
>                 }
>
> dp->v_mount can be NULL.  I think this can happen at multiple places
> in vfs_lookup() when it is sleeping.  Recently vn_lock() got even
> more calls to tsleep(9).

Is dereferencing `v_mount' the only problem here?  If you copy the flags
earlier can you trigger a different crash?

> We can add checks like this.  It this the right approach?  Or should
> we make sure that all vnodes in vfs_lookup() are always locked and
> dounmount() has to wait until lock is released?  The latter seems
> quite complicated.

How many sleep points we would have to fix like that?  Can such check be
placed at a deeper place?

> Syzcaller creates a crash at the same location, but I think it is
> not using unmount.  Don't know if it is related.
> https://syzkaller.appspot.com/bug?id=49b54eec7f3e2afc637cf49adf31d04dcca8b69e
>
> bluhm
>
> Index: kern/vfs_lookup.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_lookup.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 vfs_lookup.c
> --- kern/vfs_lookup.c 29 Jul 2019 12:35:19 -0000 1.82
> +++ kern/vfs_lookup.c 3 Sep 2019 19:16:15 -0000
> @@ -652,6 +652,12 @@ dirloop:
>   ndp->ni_vp = dp = tdp;
>   }
>
> + /* In case we were sleeping, file system may be unmounted. */
> + if (dp->v_type == VBAD) {
> + error = ENOENT;
> + goto bad2;
> + }
> +
>   /*
>   * Check for symbolic link.  Back up over any slashes that we skipped,
>   * as we will need them again.
>