>Number:         337
>Category:       kern
>Synopsis:       rename can cause kernel panic on a nullfs
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    gnats-admin (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 11 00:35:02 1994
>Originator:     Duncan McEwan
>Organization:
	Computer Science Dept, Victoria University of Wellington, New Zealand
>Release:        
>Environment:
	
	System: NetBSD orsinis.pc.comp.vuw.ac.nz 0.9C
	NetBSD 0.9C (ORSINIS) #4: Tue Jul 5 17:34:43 NZST 1994
	root@orsinis.pc.comp.vuw.ac.nz:/am/orsinis/home/src/sys/arch/i386/compile/ORSINIS i386

>Description:
	If the target file of a rename system call doesn't exist on a nullfs
	filesystem, the kernel can panic when it attempts to dereference a
	null pointer.
>How-To-Repeat:
	mkdir /tmp/foo
	mount -t null /tmp/foo /mnt
	cd /mnt
	touch bar
	mv bar baz

At this point (at least on my system) the kernel panics with

	vm_fault(f86a4000, 0, 1, 0) -> 1
	kernel: page fault trap, code=0
	stopped at	_null_bypass+0x4f:	movl 0x18(%eax), %eax

Examining the contents of eax show it to be 0.

>Fix:

The above assembler coresponds to line 253 of sys/miscfs/nullfs/null_vnops.c
($NetBSD: null_vnops.c,v 1.2 1994/06/29 06:34:35). On the 4th interation around
the enclosing loop, this_vp_p contains a pointer to a pointer to the vnode for
the destination file.  Unfortunately, when renaming to a file that doesn't
already exist, the vnode pointer is NULL, hence the panic.

My fix was to prevent dereferencing *this_vp_p if it is NULL as follows.

*** null_vnops.c.ORIG	Wed Jun 29 22:29:54 1994
--- null_vnops.c	Mon Jul 11 18:31:23 1994
***************
*** 251,258 ****
  		 * that aren't.  (We must always map first vp or vclean fails.)
  		 */
! 		if (i && (*this_vp_p)->v_op != null_vnodeop_p) {
  			old_vps[i] = NULL;
  		} else {
  			old_vps[i] = *this_vp_p;
  			*(vps_p[i]) = NULLVPTOLOWERVP(*this_vp_p);
  			/*
--- 251,259 ----
  		 * that aren't.  (We must always map first vp or vclean fails.)
  		 */
! 		if (i && *this_vp_p != 0 && (*this_vp_p)->v_op != null_vnodeop_p) {
  			old_vps[i] = NULL;
  		} else {
  			old_vps[i] = *this_vp_p;
+ 			if (! *this_vp_p) continue;
  			*(vps_p[i]) = NULLVPTOLOWERVP(*this_vp_p);
  			/*


There are other nicer ways of structuring the code to achieve the same effect,
but this was enough to convince myself that preventing the null dereference
fixed the panic.  I'll leave it up to someone in the core team to figure out if
this is the appropriate fix, and if so, what the best way of restructuring the
code is.
>Audit-Trail:
>Unformatted:


------------------------------------------------------------------------------