From owner-svn-src-stable@FreeBSD.ORG Tue Jan 5 23:33:30 2010 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4345C106566C; Tue, 5 Jan 2010 23:33:30 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 182998FC19; Tue, 5 Jan 2010 23:33:30 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o05NXTE7047272; Tue, 5 Jan 2010 23:33:29 GMT (envelope-from bz@svn.freebsd.org) Received: (from bz@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o05NXTrB047270; Tue, 5 Jan 2010 23:33:29 GMT (envelope-from bz@svn.freebsd.org) Message-Id: <201001052333.o05NXTrB047270@svn.freebsd.org> From: "Bjoern A. Zeeb" Date: Tue, 5 Jan 2010 23:33:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-6@freebsd.org X-SVN-Group: stable-6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r201625 - stable/6/sys/kern X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jan 2010 23:33:30 -0000 Author: bz Date: Tue Jan 5 23:33:29 2010 New Revision: 201625 URL: http://svn.freebsd.org/changeset/base/201625 Log: MFC r185583: Fix a credential reference leak. [1] Close subtle but relatively unlikely race conditions when propagating the vnode write error to other active sessions tracing to the same vnode, without holding a reference on the vnode anymore. [2] PR: kern/126368 [1] Submitted by: rwatson [2] Reviewed by: kib, rwatson (head) Modified: stable/6/sys/kern/kern_ktrace.c Directory Properties: stable/6/sys/ (props changed) stable/6/sys/contrib/pf/ (props changed) stable/6/sys/dev/cxgb/ (props changed) Modified: stable/6/sys/kern/kern_ktrace.c ============================================================================== --- stable/6/sys/kern/kern_ktrace.c Tue Jan 5 23:26:45 2010 (r201624) +++ stable/6/sys/kern/kern_ktrace.c Tue Jan 5 23:33:29 2010 (r201625) @@ -891,12 +891,7 @@ ktr_writerequest(struct thread *td, stru */ mtx_lock(&ktrace_mtx); vp = td->td_proc->p_tracevp; - if (vp != NULL) - VREF(vp); cred = td->td_proc->p_tracecred; - if (cred != NULL) - crhold(cred); - mtx_unlock(&ktrace_mtx); /* * If vp is NULL, the vp has been cleared out from under this @@ -905,9 +900,13 @@ ktr_writerequest(struct thread *td, stru */ if (vp == NULL) { KASSERT(cred == NULL, ("ktr_writerequest: cred != NULL")); + mtx_unlock(&ktrace_mtx); return; } + VREF(vp); KASSERT(cred != NULL, ("ktr_writerequest: cred == NULL")); + crhold(cred); + mtx_unlock(&ktrace_mtx); kth = &req->ktr_header; datalen = data_lengths[(u_short)kth->ktr_type & ~KTR_DROP]; @@ -947,18 +946,26 @@ ktr_writerequest(struct thread *td, stru error = VOP_WRITE(vp, &auio, IO_UNIT | IO_APPEND, cred); VOP_UNLOCK(vp, 0, td); vn_finished_write(mp); - vrele(vp); - mtx_unlock(&Giant); - if (!error) + crfree(cred); + if (!error) { + vrele(vp); + mtx_unlock(&Giant); return; + } + mtx_unlock(&Giant); + /* * If error encountered, give up tracing on this vnode. We defer * all the vrele()'s on the vnode until after we are finished walking * the various lists to avoid needlessly holding locks. + * NB: at this point we still hold the vnode reference that must + * not go away as we need the valid vnode to compare with. Thus let + * vrele_count start at 1 and the reference will be freed + * by the loop at the end after our last use of vp. */ log(LOG_NOTICE, "ktrace write failed, errno %d, tracing stopped\n", error); - vrele_count = 0; + vrele_count = 1; /* * First, clear this vnode from being used by any processes in the * system.