svn commit: r260541 - in head/sys/cam: . scsi
Andriy Gapon
avg at FreeBSD.org
Sat Jan 11 14:03:05 UTC 2014
on 11/01/2014 15:35 Alexander Motin said the following:
> Author: mav
> Date: Sat Jan 11 13:35:36 2014
> New Revision: 260541
> URL: http://svnweb.freebsd.org/changeset/base/260541
>
> Log:
> Take additional reference on SCSI probe periph to cover its freeze count.
>
> Otherwise periph may be invalidated and freed before single-stepping freeze
> is dropped, causing use after free panic.
Alexander,
do you think that this change will help with the panic like the following?
It occurred after I pulled out a flaky USB card reader that seemed to be in the
middle of probing attempts. The fault is a result of trying to lock a destroyed
mutex.
Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address = 0x378
fault code = supervisor read data, page not present
instruction pointer = 0x20:0xffffffff805858a0
stack pointer = 0x28:0xfffffe01de3ffa70
frame pointer = 0x28:0xfffffe01de3ffb00
code segment = base 0x0, limit 0xfffff, type 0x1b
= DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags = interrupt enabled, resume, IOPL = 0
current process = 3 (doneq0)
trap number = 12
panic: page fault
cpuid = 0
curthread: 0xfffff800112634c0
stack: 0xfffffe01de3fc000 - 0xfffffe01de400000
stack pointer: 0xfffffe01de3ff678
KDB: stack backtrace:
db_trace_self_wrapper() at 0xffffffff803adceb = db_trace_self_wrapper+0x2b/frame
0xfffffe01de3ff560
kdb_backtrace() at 0xffffffff805cbdc9 = kdb_backtrace+0x39/frame 0xfffffe01de3ff610
panic() at 0xffffffff80597783 = panic+0x1a3/frame 0xfffffe01de3ff690
trap_fatal() at 0xffffffff8074c9c2 = trap_fatal+0x3a2/frame 0xfffffe01de3ff6f0
trap_pfault() at 0xffffffff8074cbff = trap_pfault+0x22f/frame 0xfffffe01de3ff790
trap() at 0xffffffff8074c42b = trap+0x5bb/frame 0xfffffe01de3ff9b0
calltrap() at 0xffffffff80733b82 = calltrap+0x8/frame 0xfffffe01de3ff9b0
--- trap 0xc, rip = 0xffffffff805858a0, rsp = 0xfffffe01de3ffa70, rbp =
0xfffffe01de3ffb00 ---
__mtx_lock_sleep() at 0xffffffff805858a0 = __mtx_lock_sleep+0x1c0/frame
0xfffffe01de3ffb00
__mtx_lock_flags() at 0xffffffff805856c3 = __mtx_lock_flags+0x63/frame
0xfffffe01de3ffb20
xpt_done_process() at 0xffffffff8029e9ea = xpt_done_process+0x50a/frame
0xfffffe01de3ffb60
xpt_done_td() at 0xffffffff802a1896 = xpt_done_td+0x136/frame 0xfffffe01de3ffbb0
fork_exit() at 0xffffffff8056d241 = fork_exit+0x71/frame 0xfffffe01de3ffbf0
fork_trampoline() at 0xffffffff807340be = fork_trampoline+0xe/frame
0xfffffe01de3ffbf0
> Modified:
> head/sys/cam/cam_periph.c
> head/sys/cam/cam_periph.h
> head/sys/cam/cam_xpt.c
> head/sys/cam/scsi/scsi_xpt.c
>
> Modified: head/sys/cam/cam_periph.c
> ==============================================================================
> --- head/sys/cam/cam_periph.c Sat Jan 11 09:44:00 2014 (r260540)
> +++ head/sys/cam/cam_periph.c Sat Jan 11 13:35:36 2014 (r260541)
> @@ -376,6 +376,17 @@ cam_periph_acquire(struct cam_periph *pe
> }
>
> void
> +cam_periph_doacquire(struct cam_periph *periph)
> +{
> +
> + xpt_lock_buses();
> + KASSERT(periph->refcount >= 1,
> + ("cam_periph_doacquire() with refcount == %d", periph->refcount));
> + periph->refcount++;
> + xpt_unlock_buses();
> +}
> +
> +void
> cam_periph_release_locked_buses(struct cam_periph *periph)
> {
>
>
> Modified: head/sys/cam/cam_periph.h
> ==============================================================================
> --- head/sys/cam/cam_periph.h Sat Jan 11 09:44:00 2014 (r260540)
> +++ head/sys/cam/cam_periph.h Sat Jan 11 13:35:36 2014 (r260541)
> @@ -152,6 +152,7 @@ cam_status cam_periph_alloc(periph_ctor_
> struct cam_periph *cam_periph_find(struct cam_path *path, char *name);
> int cam_periph_list(struct cam_path *, struct sbuf *);
> cam_status cam_periph_acquire(struct cam_periph *periph);
> +void cam_periph_doacquire(struct cam_periph *periph);
> void cam_periph_release(struct cam_periph *periph);
> void cam_periph_release_locked(struct cam_periph *periph);
> void cam_periph_release_locked_buses(struct cam_periph *periph);
>
> Modified: head/sys/cam/cam_xpt.c
> ==============================================================================
> --- head/sys/cam/cam_xpt.c Sat Jan 11 09:44:00 2014 (r260540)
> +++ head/sys/cam/cam_xpt.c Sat Jan 11 13:35:36 2014 (r260541)
> @@ -3156,9 +3156,7 @@ restart:
> }
> if (periph->flags & CAM_PERIPH_RUN_TASK)
> break;
> - xpt_lock_buses();
> - periph->refcount++; /* Unconditionally acquire */
> - xpt_unlock_buses();
> + cam_periph_doacquire(periph);
> periph->flags |= CAM_PERIPH_RUN_TASK;
> taskqueue_enqueue(xsoftc.xpt_taskq,
> &periph->periph_run_task);
>
> Modified: head/sys/cam/scsi/scsi_xpt.c
> ==============================================================================
> --- head/sys/cam/scsi/scsi_xpt.c Sat Jan 11 09:44:00 2014 (r260540)
> +++ head/sys/cam/scsi/scsi_xpt.c Sat Jan 11 13:35:36 2014 (r260541)
> @@ -888,12 +888,14 @@ again:
> /*timeout*/60 * 1000);
> break;
> }
> +done:
> /*
> * We'll have to do without, let our probedone
> * routine finish up for us.
> */
> start_ccb->csio.data_ptr = NULL;
> cam_freeze_devq(periph->path);
> + cam_periph_doacquire(periph);
> probedone(periph, start_ccb);
> return;
> }
> @@ -919,14 +921,7 @@ again:
> /*timeout*/60 * 1000);
> break;
> }
> - /*
> - * We'll have to do without, let our probedone
> - * routine finish up for us.
> - */
> - start_ccb->csio.data_ptr = NULL;
> - cam_freeze_devq(periph->path);
> - probedone(periph, start_ccb);
> - return;
> + goto done;
> }
> case PROBE_SERIAL_NUM:
> {
> @@ -959,19 +954,13 @@ again:
> /*timeout*/60 * 1000);
> break;
> }
> - /*
> - * We'll have to do without, let our probedone
> - * routine finish up for us.
> - */
> - start_ccb->csio.data_ptr = NULL;
> - cam_freeze_devq(periph->path);
> - probedone(periph, start_ccb);
> - return;
> + goto done;
> }
> default:
> panic("probestart: invalid action state 0x%x\n", softc->action);
> }
> start_ccb->ccb_h.flags |= CAM_DEV_QFREEZE;
> + cam_periph_doacquire(periph);
> xpt_action(start_ccb);
> }
>
> @@ -1122,6 +1111,7 @@ probedone(struct cam_periph *periph, uni
> out:
> /* Drop freeze taken due to CAM_DEV_QFREEZE */
> cam_release_devq(path, 0, 0, 0, FALSE);
> + cam_periph_release_locked(periph);
> return;
> }
> else if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
> @@ -1697,6 +1687,7 @@ probe_device_check:
> CAM_DEBUG(periph->path, CAM_DEBUG_PROBE, ("Probe completed\n"));
> /* Drop freeze taken due to CAM_DEV_QFREEZE flag set. */
> cam_release_devq(path, 0, 0, 0, FALSE);
> + cam_periph_release_locked(periph);
> cam_periph_invalidate(periph);
> cam_periph_release_locked(periph);
> } else {
>
--
Andriy Gapon
More information about the svn-src-all
mailing list