From: dlg <dlg@openbsd.org>
Date: Sat, 24 Jul 2010 11:53:44 +0000 (+0000)
Subject: fix another race. vscsi_cmd checked if the adapter was running at
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d2628efdb901643c2fab34ba834a6bdb72308965;p=openbsd

fix another race. vscsi_cmd checked if the adapter was running at
the start and queued the command for processing by userland later.
the adapter could stop running between the check and the queue.
this merges the state and queue mutexes and combines the check and
queue ops in vscsi_cmd into the same critical section.

tweaked by and ok matthew@
---

diff --git a/sys/dev/vscsi.c b/sys/dev/vscsi.c
index ce48f495df2..fc0d89b536e 100644
--- a/sys/dev/vscsi.c
+++ b/sys/dev/vscsi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vscsi.c,v 1.15 2010/07/23 09:41:16 dlg Exp $ */
+/*	$OpenBSD: vscsi.c,v 1.16 2010/07/24 11:53:44 dlg Exp $ */
 
 /*
  * Copyright (c) 2008 David Gwynne <dlg@openbsd.org>
@@ -70,7 +70,6 @@ struct vscsi_softc {
 	struct vscsi_ccb_list	sc_ccb_i2t;
 	struct vscsi_ccb_list	sc_ccb_t2i;
 	int			sc_ccb_tag;
-	struct mutex		sc_ccb_mtx;
 	struct mutex		sc_poll_mtx;
 	struct rwlock		sc_ioc_lock;
 
@@ -103,8 +102,6 @@ struct scsi_adapter vscsi_switch = {
 	NULL
 };
 
-int		vscsi_running(struct vscsi_softc *);
-
 int		vscsi_i2t(struct vscsi_softc *, struct vscsi_ioc_i2t *);
 int		vscsi_data(struct vscsi_softc *, struct vscsi_ioc_data *, int);
 int		vscsi_t2i(struct vscsi_softc *, struct vscsi_ioc_t2i *);
@@ -144,7 +141,6 @@ vscsi_attach(struct device *parent, struct device *self, void *aux)
 
 	TAILQ_INIT(&sc->sc_ccb_i2t);
 	TAILQ_INIT(&sc->sc_ccb_t2i);
-	mtx_init(&sc->sc_ccb_mtx, IPL_BIO);
 	mtx_init(&sc->sc_poll_mtx, IPL_BIO);
 	mtx_init(&sc->sc_sel_mtx, IPL_BIO);
 	rw_init(&sc->sc_ioc_lock, "vscsiioc");
@@ -164,18 +160,6 @@ vscsi_attach(struct device *parent, struct device *self, void *aux)
 	    &saa, scsiprint);
 }
 
-int
-vscsi_running(struct vscsi_softc *sc)
-{
-	int				running;
-
-	mtx_enter(&sc->sc_state_mtx);
-	running = (sc->sc_state == VSCSI_S_RUNNING);
-	mtx_leave(&sc->sc_state_mtx);
-
-	return (running);
-}
-
 void
 vscsi_cmd(struct scsi_xfer *xs)
 {
@@ -183,25 +167,29 @@ vscsi_cmd(struct scsi_xfer *xs)
 	struct vscsi_softc		*sc = link->adapter_softc;
 	struct vscsi_ccb		*ccb = xs->io;
 	int				polled = ISSET(xs->flags, SCSI_POLL);
-
-	if (!vscsi_running(sc)) {
-		xs->error = XS_DRIVER_STUFFUP;
-		scsi_done(xs);
-		return;
-	}
+	int				running = 1;
 
 	if (ISSET(xs->flags, SCSI_POLL) && ISSET(xs->flags, SCSI_NOSLEEP)) {
 		printf("%s: POLL && NOSLEEP for 0x%02x\n", DEVNAME(sc),
 		    xs->cmd->opcode);
 		xs->error = XS_DRIVER_STUFFUP;
 		scsi_done(xs);
-		return;
 	}
 
 	ccb->ccb_xs = xs;
-	mtx_enter(&sc->sc_ccb_mtx);
-	TAILQ_INSERT_TAIL(&sc->sc_ccb_i2t, ccb, ccb_entry);
-	mtx_leave(&sc->sc_ccb_mtx);
+
+	mtx_enter(&sc->sc_state_mtx);
+	if (sc->sc_state == VSCSI_S_RUNNING)
+		TAILQ_INSERT_TAIL(&sc->sc_ccb_i2t, ccb, ccb_entry);
+	else
+		running = 0;
+	mtx_leave(&sc->sc_state_mtx);
+
+	if (!running) {
+		xs->error = XS_DRIVER_STUFFUP;
+		scsi_done(xs);
+		return;
+	}
 
 	selwakeup(&sc->sc_sel);
 
@@ -232,11 +220,13 @@ int
 vscsi_probe(struct scsi_link *link)
 {
 	struct vscsi_softc		*sc = link->adapter_softc;
+	int				rv;
 
-	if (!vscsi_running(sc))
-		return (ENXIO);
+	mtx_enter(&sc->sc_state_mtx);
+	rv = (sc->sc_state == VSCSI_S_RUNNING) ? 0 : ENXIO;
+	mtx_leave(&sc->sc_state_mtx);
 
-	return (0);
+	return (rv);
 }
 
 int
@@ -328,11 +318,11 @@ vscsi_i2t(struct vscsi_softc *sc, struct vscsi_ioc_i2t *i2t)
 	struct scsi_xfer		*xs;
 	struct scsi_link		*link;
 
-	mtx_enter(&sc->sc_ccb_mtx);
+	mtx_enter(&sc->sc_state_mtx);
 	ccb = TAILQ_FIRST(&sc->sc_ccb_i2t);
 	if (ccb != NULL)
 		TAILQ_REMOVE(&sc->sc_ccb_i2t, ccb, ccb_entry);
-	mtx_leave(&sc->sc_ccb_mtx);
+	mtx_leave(&sc->sc_state_mtx);
 
 	if (ccb == NULL)
 		return (EAGAIN);
@@ -462,10 +452,10 @@ vscsipoll(dev_t dev, int events, struct proc *p)
 	int				revents = 0;
 
 	if (events & (POLLIN | POLLRDNORM)) {
-		mtx_enter(&sc->sc_ccb_mtx);
+		mtx_enter(&sc->sc_state_mtx);
 		if (!TAILQ_EMPTY(&sc->sc_ccb_i2t))
 			revents |= events & (POLLIN | POLLRDNORM);
-		mtx_leave(&sc->sc_ccb_mtx);
+		mtx_leave(&sc->sc_state_mtx);
 	}
 
 	if (revents == 0) {
@@ -516,10 +506,10 @@ filt_vscsiread(struct knote *kn, long hint)
 	struct vscsi_softc *sc = (struct vscsi_softc *)kn->kn_hook;
 	int event = 0;
 
-	mtx_enter(&sc->sc_ccb_mtx);
+	mtx_enter(&sc->sc_state_mtx);
 	if (!TAILQ_EMPTY(&sc->sc_ccb_i2t))
 		event = 1;
-	mtx_leave(&sc->sc_ccb_mtx);
+	mtx_leave(&sc->sc_state_mtx);
 
 	return (event);
 }