Date: Sun, 10 Aug 2008 01:47:17 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: "Bjoern A. Zeeb" <bz@FreeBSD.org> Cc: freebsd-jail@freebsd.org Subject: Re: kern/126368: Running ktrace/kdump in jail leads to stale jails Message-ID: <20080809234717.GC13799@skucha.home.aster.pl> In-Reply-To: <20080808184224.H88849@maildrop.int.zabbadoz.net> References: <200808081740.m78He4bc084276@freefall.freebsd.org> <20080808184224.H88849@maildrop.int.zabbadoz.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--HcAYCG3uE/tztfnV Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline On Fri, Aug 08, 2008 at 06:43:38PM +0000, Bjoern A. Zeeb wrote: > >The following reply was made to PR kern/126368; it has been noted by GNATS. > > > >From: "Mateusz Guzik" <mjguzik@gmail.com> > >To: bug-followup@freebsd.org > >Cc: > >Subject: Re: kern/126368: Running ktrace/kdump in jail leads to stale jails > >Date: Fri, 8 Aug 2008 19:30:22 +0200 > > > >Err, I made a mistake. crfree() will be called in case of failure > >(loop starting at line 959), so the following patch should be ok: > > > >--- sys/kern/kern_ktrace.c.orig 2008-08-08 16:37:45.000000000 +0200 > >+++ sys/kern/kern_ktrace.c 2008-08-08 19:25:16.000000000 +0200 > >@@ -933,12 +933,14 @@ > > error = VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, cred); > > VOP_UNLOCK(vp, 0, td); > > vn_finished_write(mp); > > vrele(vp); > > VFS_UNLOCK_GIANT(vfslocked); > >- if (!error) > >+ if (!error) { > >+ crfree(cred); > > return; > >+ } > > that sounds more plausible w/o seeing the surrounding code. I had > wondered already earlier today when I was pointed at. > > I'll look into this. > Sorry for the noise -- the first patch was right. ;) ktr_writerequest() is called multiple times and it _always_ calls crhold(), so crfree() must be called before it returns (even in case of failure). Also, in this function one can find: [..] crhold(cred) [..] if (vp == NULL) { KASSERT(cred == NULL, ("ktr_writerequest: cred != NULL")); return; } `Normal' kernel might leak credentials in this case, so I believe crfree() should be added there too. Thanks, and again, sorry for the noise. -- Mateusz Guzik --HcAYCG3uE/tztfnV Content-Type: text/x-diff; charset=iso-8859-2 Content-Disposition: attachment; filename="kern_ktrace.diff" --- sys/kern/kern_ktrace.c.orig 2008-08-08 16:37:45.000000000 +0200 +++ sys/kern/kern_ktrace.c 2008-08-10 01:42:07.000000000 +0200 @@ -889,10 +889,12 @@ * request, so just drop it. Make sure the credential and vnode are * in sync: we should have both or neither. */ if (vp == NULL) { KASSERT(cred == NULL, ("ktr_writerequest: cred != NULL")); + if (cred != NULL) + crfree(cred); return; } KASSERT(cred != NULL, ("ktr_writerequest: cred == NULL")); kth = &req->ktr_header; @@ -933,10 +935,11 @@ error = VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, cred); VOP_UNLOCK(vp, 0, td); vn_finished_write(mp); vrele(vp); VFS_UNLOCK_GIANT(vfslocked); + crfree(cred); if (!error) return; /* * If error encountered, give up tracing on this vnode. We defer * all the vrele()'s on the vnode until after we are finished walking --HcAYCG3uE/tztfnV--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080809234717.GC13799>