[src/trunk]: src/sys/kern Take a reference on ndp->ni_rootdir and ndp->ni_ero...

[hannken <hannken%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/14d676604fc0
branches:  trunk
changeset: 997551:14d676604fc0
user:      hannken <hannken%NetBSD.org@localhost>
date:      Tue Mar 12 14:03:35 2019 +0000

description:
Take a reference on ndp->ni_rootdir and ndp->ni_erootdir.

A multithreaded process may chroot during namei() and we end up with
vn_under() trying to reference the now unreferenced ni_rootdir.

Ok: David Holland <dholland%netbsd.org@localhost>

Reported-by: syzbot+889319cdf91a3d0373a9%syzkaller.appspotmail.com@localhost

diffstat:

 sys/kern/vfs_lookup.c |  28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

diffs (79 lines):

diff -r ad592db646da -r 14d676604fc0 sys/kern/vfs_lookup.c
--- a/sys/kern/vfs_lookup.c     Tue Mar 12 11:05:09 2019 +0000
+++ b/sys/kern/vfs_lookup.c     Tue Mar 12 14:03:35 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_lookup.c,v 1.208 2017/07/09 22:48:44 dholland Exp $        */
+/*     $NetBSD: vfs_lookup.c,v 1.209 2019/03/12 14:03:35 hannken Exp $ */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.208 2017/07/09 22:48:44 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.209 2019/03/12 14:03:35 hannken Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_magiclinks.h"
@@ -469,6 +469,8 @@
        int slashes;
 
        unsigned attempt_retry:1;       /* true if error allows emul retry */
+       unsigned root_referenced:1;     /* true if ndp->ni_rootdir and
+                                            ndp->ni_erootdir were referenced */
 };
 
 
@@ -486,6 +488,8 @@
        state->rdonly = 0;
        state->slashes = 0;
 
+       state->root_referenced = 0;
+
        KASSERTMSG((state->cnp->cn_cred != NULL), "namei: bad cred/proc");
        KASSERTMSG(((state->cnp->cn_nameiop & (~OPMASK)) == 0),
            "namei: nameiop contaminated with flags: %08"PRIx32,
@@ -510,8 +514,11 @@
 {
        KASSERT(state->cnp == &state->ndp->ni_cnd);
 
-       /* nothing for now */
-       (void)state;
+       if (state->root_referenced) {
+               vrele(state->ndp->ni_rootdir);
+               if (state->ndp->ni_erootdir != NULL)
+                       vrele(state->ndp->ni_erootdir);
+       }
 }
 
 //////////////////////////////
@@ -578,11 +585,15 @@
        /*
         * Get a reference to the start dir so we can safely unlock cwdi.
         *
-        * XXX: should we hold references to rootdir and erootdir while
-        * we're running? What happens if a multithreaded process chroots
-        * during namei?
+        * Must hold references to rootdir and erootdir while we're running.
+        * A multithreaded process may chroot during namei.
         */
        vref(startdir);
+       KASSERT(! state->root_referenced);
+       vref(state->ndp->ni_rootdir);
+       if (state->ndp->ni_erootdir != NULL)
+               vref(state->ndp->ni_erootdir);
+       state->root_referenced = 1;
 
        rw_exit(&cwdi->cwdi_lock);
        return startdir;
@@ -603,6 +614,9 @@
        state->ndp->ni_erootdir = NULL;
 
        vref(state->ndp->ni_atdir);
+       KASSERT(! state->root_referenced);
+       vref(state->ndp->ni_rootdir);
+       state->root_referenced = 1;
        return state->ndp->ni_atdir;
 }