Bug 46176 - [umass] [panic] umass causes kernel panic if device removed before umount
Summary: [umass] [panic] umass causes kernel panic if device removed before umount
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-12-10 23:20 UTC by Frode Nordahl
Modified: 2009-04-13 16:50 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frode Nordahl 2002-12-10 23:20:00 UTC
Using a JMTek USBDRIVE.  If the usbdrive is removed before unmounting the
filesystem, forcing a unmount later and then reinserting the drive will cause
a kernel panic.

This is 100% reproducible.

How-To-Repeat: First attach:
umass0: Luwen EasyDisc, rev 1.10/1.00, addr 3
da0 at umass-sim0 bus 0 target 0 lun 0
da0: <JMTek USBDRIVE 1.00> Removable Direct Access SCSI-2 device
da0: 1.000MB/s transfers
da0: 15MB (32000 512 byte sectors: 64H 32S/T 15C)
(da0:umass-sim0:0:0:0): READ(6)/WRITE(6) not supported, increasing minimum_cmd_s
ize to 10.

Mount:
mount -t msdos /dev/da0s1 /mnt

Detach:
umass0: at uhub0 port 1 (addr 3) disconnected
(da0:umass-sim0:0:0:0): lost device
umass0: detached

Umount:
# umount /mnt
umass-sim:0:0:0:func_code 0x0901: Invalid target (target needed)
(da0:umass-sim0:0:0:0): Synchronize cache failed, status == 0x39, scsi status ==
 0x0
umass-sim:0:0:0:func_code 0x0901: Invalid target (target needed)
(da0:umass-sim0:0:0:0): removing device entry

Reattach the device:
Fatal trap 12: page fault while in kernel mode
fault virtual address   = 0x0
fault code              = supervisor read, page not present
instruction pointer     = 0x8:0xc7001005
stack pointer           = 0x10:0xcc8bf864
frame pointer           = 0x10:0xcc8bf8b0
code segment            = base rx0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, def32 1, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 482 (mount)

Backtrace:
(kgdb) bt
#0  0xc0248cab in doadump ()
#1  0xc02491c9 in boot ()
#2  0xc0249413 in panic ()
#3  0xc0147ef2 in db_panic ()
#4  0xc0147e72 in db_command ()
#5  0xc0147f86 in db_command_loop ()
#6  0xc014ac9a in db_trap ()
#7  0xc03bcbf2 in kdb_trap ()
#8  0xc03ce2f2 in trap_fatal ()
#9  0xc03ce002 in trap_pfault ()
#10 0xc03cdaf0 in trap ()
#11 0xc03be3d8 in calltrap ()
#12 0xc020fc47 in g_dev_open ()
#13 0xc020d915 in spec_open ()
#14 0xc020d698 in spec_vnoperate ()
#15 0xc03718cc in ffs_mountfs ()
#16 0xc0370e65 in ffs_mount ()
#17 0xc029a8b8 in vfs_mount ()
#18 0xc0299f68 in mount ()
#19 0xc03ce64a in syscall ()
#20 0xc03be42d in Xint0x80_syscall ()


Other usefull info:
If I do not unmount the filesystem after the first removal, reinserting the
device instead the following happends:

First reattach w/mount w/o unmount:
uhub0: port error, restarting port 1
umass0: Luwen EasyDisc, rev 1.10/1.00, addr 3

Following attaches:
uhub0: port error, restarting port 1
umass0: Luwen EasyDisc, rev 1.10/1.00, addr 3
cam_periph_alloc: attempt to re-allocate valid device da0 rejected
daasync: Unable to attach to new device due to status 0x6
Comment 1 Frode Nordahl 2002-12-15 08:42:39 UTC
Sorry about the bogus backtrace in the original PR.

Here is a new one:

#0  doadump () at ../../../kern/kern_shutdown.c:232
232             dumping++;
(kgdb) bt
#0  doadump () at ../../../kern/kern_shutdown.c:232
#1  0xc02491c9 in boot (howto=260) at ../../../kern/kern_shutdown.c:364
#2  0xc0249413 in panic () at ../../../kern/kern_shutdown.c:517
#3  0xc0147ef2 in db_panic () at ../../../ddb/db_command.c:450
#4  0xc0147e72 in db_command (last_cmdp=0xc042ed40, cmd_table=0x0,
aux_cmd_tablep=0xc04281b4, aux_cmd_tablep_end=0xc04281b8)
    at ../../../ddb/db_command.c:346
#5  0xc0147f86 in db_command_loop () at ../../../ddb/db_command.c:472
#6  0xc014ac9a in db_trap (type=12, code=0) at ../../../ddb/db_trap.c:72
#7  0xc03bcbf2 in kdb_trap (type=12, code=0, regs=0xcc8bf824) at
../../../i386/i386/db_interface.c:166
#8  0xc03ce2f2 in trap_fatal (frame=0xcc8bf824, eva=0) at
../../../i386/i386/trap.c:839
#9  0xc03ce002 in trap_pfault (frame=0xcc8bf824, usermode=0, eva=0) at
../../../i386/i386/trap.c:758
#10 0xc03cdaf0 in trap (frame=
      {tf_fs = 24, tf_es = -1071710192, tf_ds = 16, tf_edi =
-1035469184, tf_esi = -1034535936, tf_ebp = -863242064, tf_isp =
-863242160, tf_ebx = -1037456704, tf_edx = 1, tf_ecx = 0, tf_eax =
-1072438608, tf_trapno = 12, tf_err = 0, tf_eip = -956297211, tf_cs = 8,
tf_eflags = 66178, tf_esp = -863242064, tf_ss = -1071559308}) at
../../../i386/i386/trap.c:445
#11 0xc03be3d8 in calltrap () at {standard input}:98
#12 0xc020fc47 in g_dev_open (dev=0x0, flags=-1035469184, fmt=0, td=0x0)
at ../../../geom/geom_dev.c:201
#13 0xc020d915 in spec_open (ap=0xcc8bf9c8) at
../../../fs/specfs/spec_vnops.c:208
#14 0xc020d698 in spec_vnoperate (ap=0x0) at
../../../fs/specfs/spec_vnops.c:126
#15 0xc03718cc in ffs_mountfs (devvp=0xc2564384, mp=0xc2574a00,
td=0xc21fd8c0, malloctype=0x0) at vnode_if.h:213
#16 0xc0370e65 in ffs_mount (mp=0xc2574a00, path=0xc256ff00 "/mnt",
data=0x0, ndp=0xc2564384, td=0xc21fd8c0)
    at ../../../ufs/ffs/ffs_vfsops.c:353
#17 0xc029a8b8 in vfs_mount (td=0xc21fd8c0, fstype=0x0,
fspath=0xc256ff00 "/mnt", fsflags=-1034534012, fsdata=0x0)
    at ../../../kern/vfs_mount.c:1061
#18 0xc0299f68 in mount (td=0x0, uap=0xcc8bfd10) at
../../../kern/vfs_mount.c:818
#19 0xc03ce64a in syscall (frame=
      {tf_fs = 47, tf_es = 47, tf_ds = 47, tf_edi = 134850899, tf_esi =
-1077937628, tf_ebp = -1077940008, tf_isp = -863240844, tf_ebx =
-1077939952, tf_edx = 0, tf_ecx = 0, tf_eax = 21, tf_trapno = 12, tf_err
= 2, tf_eip = 134525795, tf_cs = 31, tf_eflags = 582, tf_esp =
-1077940164, tf_ss = 47}) at ../../../i386/i386/trap.c:1033
#20 0xc03be42d in Xint0x80_syscall () at {standard input}:140


Mvh,
Frode Nordahl
Comment 2 Kris Kennaway freebsd_committer freebsd_triage 2003-07-18 01:55:28 UTC
Responsible Changed
From-To: freebsd-bugs->joe

Assign to USB maintainer
Comment 3 joe freebsd_committer freebsd_triage 2004-11-10 10:59:01 UTC
Responsible Changed
From-To: joe->freebsd-usb

Hand this over to the usb mailling list.
Comment 4 dfilter service freebsd_committer freebsd_triage 2007-12-27 16:38:34 UTC
imp         2007-12-27 16:38:29 UTC

  FreeBSD src repository

  Modified files:
    sys/kern             vfs_bio.c vfs_mount.c 
  Log:
  A partial solution to some of the 'pull the umass device with a
  mounted FS' problems.  These are more along the lines of 'avoiding an
  avoidable panic' than a complete solution to removable devices.  We
  now close the barn door after the horse has gotten lose and has been
  hit by a truck, as it were.  The barn no longer catches fire in this
  case, but the horse is still dead :-).
  
  The vfs_bio.c fix causes us not to put a failed write back into the
  dirty pool if the error returned was ENXIO.  In that case, the buffer
  is treated like any other clean buffer that's being retured.  ENXIO
  means the device isn't there anymore and will never be there again in
  the future, so retrying is futile.
  
  The vfs_mount.c fix treats 'ENXIO' as success for unmounting a file
  system.  If the device is gone, retrying later won't help and we'll
  never be able to unmount the device.
  
  These two are part of a larger patch set submitted by the author.  The
  other patches will be forth coming.  I added comments to these two
  patches.
  
  Submitted by: Henrik Gulbrandsen
  Reviewed by: phk@
  PR: usb/46176 (partial)
  
  Revision  Changes    Path
  1.531     +4 -0      src/sys/kern/vfs_bio.c
  1.269     +7 -2      src/sys/kern/vfs_mount.c
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 5 Mark Linimon freebsd_committer freebsd_triage 2008-02-29 00:44:41 UTC
State Changed
From-To: open->patched

imp has committed a partial fix (the full fix will require more work); 
it still needs an MFC. 


Comment 6 Mark Linimon freebsd_committer freebsd_triage 2008-02-29 00:44:41 UTC
Responsible Changed
From-To: freebsd-usb->imp
Comment 7 dfilter service freebsd_committer freebsd_triage 2009-03-01 10:51:55 UTC
Author: trasz
Date: Sun Mar  1 10:51:34 2009
New Revision: 189224
URL: http://svn.freebsd.org/changeset/base/189224

Log:
  MFC r174937 by imp.
  
  Reviewed by:	imp
  Approved by:	rwatson (mentor)
  
  Original commit log:
  
  A partial solution to some of the 'pull the umass device with a
  mounted FS' problems.  These are more along the lines of 'avoiding an
  avoidable panic' than a complete solution to removable devices.  We
  now close the barn door after the horse has gotten lose and has been
  hit by a truck, as it were.  The barn no longer catches fire in this
  case, but the horse is still dead :-).
  
  The vfs_bio.c fix causes us not to put a failed write back into the
  dirty pool if the error returned was ENXIO.  In that case, the buffer
  is treated like any other clean buffer that's being retured.  ENXIO
  means the device isn't there anymore and will never be there again in
  the future, so retrying is futile.
  
  The vfs_mount.c fix treats 'ENXIO' as success for unmounting a file
  system.  If the device is gone, retrying later won't help and we'll
  never be able to unmount the device.
  
  These two are part of a larger patch set submitted by the author.  The
  other patches will be forth coming.  I added comments to these two
  patches.
  
  Submitted by: Henrik Gulbrandsen
  Reviewed by: phk@
  PR: usb/46176 (partial)

Modified:
  stable/7/sys/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)
  stable/7/sys/dev/ath/ath_hal/   (props changed)
  stable/7/sys/dev/cxgb/   (props changed)
  stable/7/sys/kern/vfs_bio.c
  stable/7/sys/kern/vfs_mount.c

Modified: stable/7/sys/kern/vfs_bio.c
==============================================================================
--- stable/7/sys/kern/vfs_bio.c	Sun Mar  1 09:51:50 2009	(r189223)
+++ stable/7/sys/kern/vfs_bio.c	Sun Mar  1 10:51:34 2009	(r189224)
@@ -1170,6 +1170,7 @@ brelse(struct buf *bp)
 
 	if (bp->b_iocmd == BIO_WRITE &&
 	    (bp->b_ioflags & BIO_ERROR) &&
+	    bp->b_error != ENXIO &&
 	    !(bp->b_flags & B_INVAL)) {
 		/*
 		 * Failed write, redirty.  Must clear BIO_ERROR to prevent
@@ -1177,6 +1178,9 @@ brelse(struct buf *bp)
 		 * this case is not run and the next case is run to 
 		 * destroy the buffer.  B_INVAL can occur if the buffer
 		 * is outside the range supported by the underlying device.
+		 * If the error is that the device went away (ENXIO), we
+		 * shouldn't redirty the buffer either, but discard the
+		 * data too.
 		 */
 		bp->b_ioflags &= ~BIO_ERROR;
 		bdirty(bp);

Modified: stable/7/sys/kern/vfs_mount.c
==============================================================================
--- stable/7/sys/kern/vfs_mount.c	Sun Mar  1 09:51:50 2009	(r189223)
+++ stable/7/sys/kern/vfs_mount.c	Sun Mar  1 10:51:34 2009	(r189224)
@@ -1293,8 +1293,13 @@ dounmount(mp, flags, td)
 		error = VFS_UNMOUNT(mp, flags, td);
 	}
 	vn_finished_write(mp);
-	if (error) {
-		/* Undo cdir/rdir and rootvnode changes made above. */
+	/*
+	 * If we failed to flush the dirty blocks for this mount point,
+	 * undo all the cdir/rdir and rootvnode changes we made above.
+	 * Unless we failed to do so because the device is reporting that
+	 * it doesn't exist anymore.
+	 */
+	if (error && error != ENXIO) {
 		if ((flags & MNT_FORCE) &&
 		    VFS_ROOT(mp, LK_EXCLUSIVE, &fsrootvp, td) == 0) {
 			if (mp->mnt_vnodecovered != NULL)
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 Warner Losh freebsd_committer freebsd_triage 2009-04-13 16:50:34 UTC
State Changed
From-To: patched->closed

These have been in a release...