Subject: kern/5357: ktrace -c can panic system
To: None <gnats-bugs@gnats.netbsd.org>
From: None <nathanw@MIT.EDU>
List: netbsd-bugs
Date: 04/24/1998 15:53:57
>Number:         5357
>Category:       kern
>Synopsis:       Any user can panic a 1.3.1 system with 'ktrace -c' with DIAGNOSTIC compiled in.
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Apr 24 13:05:01 1998
>Last-Modified:
>Originator:     Nathan J Williams
>Organization:
	Massachvsetts Institvte of Technology
>Release:        NetBSD 1.3.1
>Environment:
System: NetBSD snorklewacker.mit.edu 1.3.1 NetBSD 1.3.1 (ATHENADEBUG) #1: Fri Apr 24 14:58:34 EDT 1998 nathanw@snorklewacker.mit.edu:/u1/var/tmp/sys/arch/i386/compile/ATHENADEBUG i386


>Description:
	The sys_ktrace() syscall, when it starts a trace, opens a vnode
with flags FREAD|FWRITE, starts tracing, and closes it with FREAD|FWRITE. 
Starting tracing includes calling VREF(), which increments the v_usecount
field. Normally, when tracing is stopped, vrele() is called to release the 
node, which decrements v_usecount. All is happy. In the KTROP_CLEARFILE 
operation, however, vn_close() is called with flags FREAD|FWRITE. This 
decrements the v_writecount field as well as v_usecount. The next time 
vn_close() is called with FWRITE (either when a second process tracing to 
the same vnode is found, or just before sys_ktrace exits), the v_writecount 
field is decremented below zero. vrele() notices this and panics. 
	
>How-To-Repeat:
	Run "ktrace -p <pid>; ktrace -c <pid>" on a system built with 
"options DIAGNOSTIC" (eg, GENERIC). Watch system panic with
"vrele: bad ref cnt". (second <pid> argument isn't used, but ktrace requires 
it. see next pr...)
>Fix:
	Simple fix: Release vnode with vrele() rather than vn_close() 
when handling KTROP_CLEARFILE. This works and is the way the other cases
(both in kern_ktrace.c and kern_exit.c) dispose of tracefile vnodes. Patch
provided below.

	More complicated fix: Determine if the v_writecount field should
be maintained for ktracing and if so change VREF() calls to a not-yet-existent
call (VWREF?) to increment the v_usecount and v_writecount fields, and 
replace the vrele() calls with vn_close() calls. I'm not sure of the 
correctness or feasablilty of this approach (especially what the allowed 
side effects of vn_close are, and whether they're desireable), so I avoided it.

*** kern_ktrace.c.orig	Fri Apr 24 14:34:26 1998
--- kern_ktrace.c	Fri Apr 24 14:54:33 1998
***************
*** 308,315 ****
  				if (ktrcanset(curp, p)) {
  					p->p_tracep = NULL;
  					p->p_traceflag = 0;
! 					(void) vn_close(vp, FREAD|FWRITE,
! 						p->p_ucred, p);
  				} else
  					error = EPERM;
  			}
--- 308,314 ----
  				if (ktrcanset(curp, p)) {
  					p->p_tracep = NULL;
  					p->p_traceflag = 0;
! 					vrele(vp);
  				} else
  					error = EPERM;
  			}
>Audit-Trail:
>Unformatted: