NetBSD Problem Report #337

From gnats  Mon Jul 11 00:31:53 1994
Received: from kaukau.comp.vuw.ac.nz (kaukau.comp.vuw.ac.nz [130.195.5.20]) by sun-lamp.cs.berkeley.edu (8.6.9/8.6.9) with ESMTP id AAA10009 for <gnats-bugs@sun-lamp.cs.berkeley.edu>; Mon, 11 Jul 1994 00:31:49 -0700
Message-Id: <199407110736.TAA00565@orsinis.pc.comp.vuw.ac.nz>
Date: Mon, 11 Jul 1994 19:36:50 +1200
From: Duncan McEwan <duncan@Comp.VUW.AC.NZ>
Reply-To: duncan@Comp.VUW.AC.NZ
To: gnats-bugs@sun-lamp.cs.berkeley.edu
Subject: rename can cause kernel panic on a nullfs
X-Send-Pr-Version: 3.2

>Number:         337
>Category:       kern
>Synopsis:       rename can cause kernel panic on a nullfs
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    gnats-admin
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 11 00:35:02 +0000 1994
>Closed-Date:    Wed Jul 20 07:40:43 +0000 1994
>Last-Modified:  
>Originator:     Duncan McEwan
>Release:        
>Organization:
	Computer Science Dept, Victoria University of Wellington, New Zealand
>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.
>Release-Note:
>Audit-Trail:
State-Changed-From-To: open->closed 
State-Changed-By: mycroft 
State-Changed-When: Wed Jul 20 00:40:43 PDT 1994 
State-Changed-Why:  
Fixed. 
>Unformatted:

Home	PR Database Search