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" <[EMAIL PROTECTED]>
> >To: [EMAIL PROTECTED]
> >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
--- 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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-jail
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to