Re: kern/46592: lib/libc/sys/t_mmap cause kernel panic

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR kern/46592; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/46592: lib/libc/sys/t_mmap cause kernel panic
Date: Thu, 14 Jun 2012 18:56:05 +0200

 On Tue, Jun 12, 2012 at 10:05:00AM +0000, bouyer%antioche.eu.org@localhost 
wrote:
 >      As shown in automated run reports at:
 >      http://www-soc.lip6.fr/~bouyer/NetBSD-tests/xen/HEAD/
 >      amd64 and i386 kernels have been panicing on a t_mmap test for some
 >      time.
 > lib/libc/sys/t_mmap (332/519): 8 test cases
 >     mmap_block: panic: kernel diagnostic assertion "vp == 
 > vp->v_specnode->sn_dev->sd_bdevvp" failed: file 
 > "/home/builds/ab/HEAD/src/sys/miscfs/specfs/spec_vnops.c", line 891 
 > cpu1: Begin traceback...
 
 This test does 2 reads at the mmaped memory: one before closing the
 file descriptor used to mmap() the device, and one after closing.
 It's the second read which causes the panic.
 
 > kern_assert() at netbsd:kern_assert+0xae
 > spec_strategy() at netbsd:spec_strategy+0x97
 > VOP_STRATEGY() at netbsd:VOP_STRATEGY+0x33
 > genfs_getpages() at netbsd:genfs_getpages+0x1161
 > VOP_GETPAGES() at netbsd:VOP_GETPAGES+0x4f
 > uvn_get() at netbsd:uvn_get+0x50
 > uvm_fault_internal() at netbsd:uvm_fault_internal+0xcab
 > trap() at netbsd:trap+0x4e4
 
 I found the problem:
 on the native anita run, the device used for tests is /dev/wd0d; on
 xen anita run it is /dev/xbd0d.
 
 With the native run, the panic doesn't happen because in genfs_getpage()
 vp->v_size is 0, so it doesn't go to VOP_STRATEGY() and the process gets
 a SEGV. With the Xen run vp->v_size is set to the size of the device.
 
 The problem is in spec_open(), which does a uvm_vnp_setsize() at the end.
 The problem is there:
         if (cdev_type(dev) != D_DISK || error != 0)  
                return error;
 But if dev is not a character device, cdev_type(dev) won't return the
 right value. For wd0d it returns D_TTY:
 brw-r-----  1 root  operator  0, 3 May  3  2011 /dev/wd0d
 crw-r-----  1 root  operator  3, 3 May  3  2011 /dev/rwd0d
 crw-------  1 root    wheel       0,       0 Jun 14 13:51 console
 crw-------  1 root    wheel       0,       1 Jun 14 18:50 constty
 
 cdev_type(dev) looks at the cons driver instead of wd in this case.
 But for xbd0d it returns D_DISK, because the major is the same for
 block and char:
 brw-r-----  1 root  operator  142, 3 May  3  2011 /dev/xbd0d
 crw-r-----  1 root  operator  142, 3 May  3  2011 /dev/rxbd0d
 
 and so spec_open() goes to the end, where it does a uvm_vnp_setsize().
 
 A quick fix would be:
 Index: miscfs/specfs/spec_vnops.c
 ===================================================================
 RCS file: /cvsroot/src/sys/miscfs/specfs/spec_vnops.c,v
 retrieving revision 1.135
 diff -u -p -u -r1.135 spec_vnops.c
 --- miscfs/specfs/spec_vnops.c 29 Apr 2012 22:54:00 -0000      1.135
 +++ miscfs/specfs/spec_vnops.c 14 Jun 2012 16:53:09 -0000
 @@ -541,7 +541,7 @@ spec_open(void *v)
        }
        mutex_exit(&device_lock);
  
 -      if (cdev_type(dev) != D_DISK || error != 0)
 +      if (vp->v_type != VCHR || cdev_type(dev) != D_DISK || error != 0)
                return error;
  
        if (vp->v_type == VCHR)
 
 but it looks like the intend of the code was to set the size for both block and
 char disk devices, so I'm not sure what the right thing to do here really is.
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --