From owner-freebsd-fs@freebsd.org Thu Jul 9 10:16:14 2015 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A8D33996EEF for ; Thu, 9 Jul 2015 10:16:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 370F21F94; Thu, 9 Jul 2015 10:16:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t69AG4nZ011653 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 9 Jul 2015 13:16:04 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t69AG4nZ011653 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t69AG47N011617; Thu, 9 Jul 2015 13:16:04 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 9 Jul 2015 13:16:04 +0300 From: Konstantin Belousov To: Mateusz Guzik Cc: rwatson@FreeBSD.org, freebsd-fs@freebsd.org, Mateusz Guzik Subject: Re: [PATCH 1/4] vfs: plug a use-after-free of fd_rdir in namei Message-ID: <20150709101604.GM2080@kib.kiev.ua> References: <20150707085857.GZ2080@kib.kiev.ua> <1436393231-5831-1-git-send-email-mjguzik@gmail.com> <1436393231-5831-2-git-send-email-mjguzik@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436393231-5831-2-git-send-email-mjguzik@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jul 2015 10:16:14 -0000 On Thu, Jul 09, 2015 at 12:07:08AM +0200, Mateusz Guzik wrote: > From: Mateusz Guzik > > fd_rdir vnode was stored in ni_rootdir without refing it in any way, > after which the filedsc lock was being dropped. > > The vnode could have been freed by mountcheckdirs or another thread doing > chroot. > > VREF the vnode while the lock is held. Patch looks fine. Would it make sense to extend namei_cleanup to also handle deref ? > > MFC after: 1 week > --- > sys/kern/vfs_lookup.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c > index 5dc07dc..20f8e96 100644 > --- a/sys/kern/vfs_lookup.c > +++ b/sys/kern/vfs_lookup.c > @@ -210,6 +210,7 @@ namei(struct nameidata *ndp) > */ > FILEDESC_SLOCK(fdp); > ndp->ni_rootdir = fdp->fd_rdir; > + VREF(ndp->ni_rootdir); > ndp->ni_topdir = fdp->fd_jdir; > > /* > @@ -260,6 +261,7 @@ namei(struct nameidata *ndp) > } > } > if (error) { > + vrele(ndp->ni_rootdir); > namei_cleanup_cnp(cnp); > return (error); > } > @@ -286,6 +288,7 @@ namei(struct nameidata *ndp) > if (KTRPOINT(curthread, KTR_CAPFAIL)) > ktrcapfail(CAPFAIL_LOOKUP, NULL, NULL); > #endif > + vrele(ndp->ni_rootdir); > namei_cleanup_cnp(cnp); > return (ENOTCAPABLE); > } > @@ -299,6 +302,7 @@ namei(struct nameidata *ndp) > ndp->ni_startdir = dp; > error = lookup(ndp); > if (error) { > + vrele(ndp->ni_rootdir); > namei_cleanup_cnp(cnp); > SDT_PROBE(vfs, namei, lookup, return, error, NULL, 0, > 0, 0); > @@ -308,6 +312,7 @@ namei(struct nameidata *ndp) > * If not a symbolic link, we're done. > */ > if ((cnp->cn_flags & ISSYMLINK) == 0) { > + vrele(ndp->ni_rootdir); > if ((cnp->cn_flags & (SAVENAME | SAVESTART)) == 0) { > namei_cleanup_cnp(cnp); > } else > @@ -371,6 +376,7 @@ namei(struct nameidata *ndp) > vput(ndp->ni_vp); > dp = ndp->ni_dvp; > } > + vrele(ndp->ni_rootdir); > namei_cleanup_cnp(cnp); > vput(ndp->ni_vp); > ndp->ni_vp = NULL; > -- > 2.4.5