When accessing the scsi link of a scsi disk, use a variable "sc_link"

everywhere.  This is the first step to fix a use after free of the
sc_link when the disk detaches.  If a function gets a scsi transfer,
the scsi link is always valid.  Call this variable "link" consistently.
OK krw@

1 files changed, 81 insertions, 61 deletions

diff --git a/sys/scsi/sd.c b/sys/scsi/sd.c
index 142f3aad48e..96c6bd815a1 100644
--- a/sys/scsi/sd.c
+++ b/sys/scsi/sd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sd.c,v 1.261 2015/06/07 19:13:27 krw Exp $	*/
+/*	$OpenBSD: sd.c,v 1.262 2016/02/03 15:16:33 bluhm Exp $	*/
 /*	$NetBSD: sd.c,v 1.111 1997/04/02 02:29:41 mycroft Exp $	*/
 
 /*-
@@ -199,7 +199,7 @@ sdattach(struct device *parent, struct device *self, void *aux)
 	    &sc->sc_xsh);
 
 	/* Spin up non-UMASS devices ready or not. */
-	if ((sc->sc_link->flags & SDEV_UMASS) == 0)
+	if ((sc_link->flags & SDEV_UMASS) == 0)
 		scsi_start(sc_link, SSS_START, sd_autoconf);
 
 	/*
@@ -374,7 +374,7 @@ sdopen(dev_t dev, int flag, int fmt, struct proc *p)
 		}
 	} else {
 		/* Spin up non-UMASS devices ready or not. */
-		if ((sc->sc_link->flags & SDEV_UMASS) == 0)
+		if ((sc_link->flags & SDEV_UMASS) == 0)
 			scsi_start(sc_link, SSS_START, (rawopen ? SCSI_SILENT :
 			    0) | SCSI_IGNORE_ILLEGAL_REQUEST |
 			    SCSI_IGNORE_MEDIA_CHANGE);
@@ -438,7 +438,7 @@ out:
 	/* It's OK to fall through because dk_openmask is now non-zero. */
 bad:
 	if (sc->sc_dk.dk_openmask == 0) {
-		if ((sc->sc_link->flags & SDEV_REMOVABLE) != 0)
+		if ((sc_link->flags & SDEV_REMOVABLE) != 0)
 			scsi_prevent(sc_link, PR_ALLOW, SCSI_SILENT |
 			    SCSI_IGNORE_ILLEGAL_REQUEST |
 			    SCSI_IGNORE_MEDIA_CHANGE);
@@ -457,6 +457,7 @@ bad:
 int
 sdclose(dev_t dev, int flag, int fmt, struct proc *p)
 {
+	struct scsi_link *sc_link;
 	struct sd_softc *sc;
 	int part = DISKPART(dev);
 
@@ -467,6 +468,7 @@ sdclose(dev_t dev, int flag, int fmt, struct proc *p)
 		device_unref(&sc->sc_dev);
 		return (ENXIO);
 	}
+	sc_link = sc->sc_link;
 
 	disk_lock_nointr(&sc->sc_dk);
 
@@ -476,15 +478,15 @@ sdclose(dev_t dev, int flag, int fmt, struct proc *p)
 		if ((sc->flags & SDF_DIRTY) != 0)
 			sd_flush(sc, 0);
 
-		if ((sc->sc_link->flags & SDEV_REMOVABLE) != 0)
-			scsi_prevent(sc->sc_link, PR_ALLOW,
+		if ((sc_link->flags & SDEV_REMOVABLE) != 0)
+			scsi_prevent(sc_link, PR_ALLOW,
 			    SCSI_IGNORE_ILLEGAL_REQUEST |
 			    SCSI_IGNORE_NOT_READY | SCSI_SILENT);
-		sc->sc_link->flags &= ~(SDEV_OPEN | SDEV_MEDIA_LOADED);
+		sc_link->flags &= ~(SDEV_OPEN | SDEV_MEDIA_LOADED);
 
-		if (sc->sc_link->flags & SDEV_EJECTING) {
-			scsi_start(sc->sc_link, SSS_STOP|SSS_LOEJ, 0);
-			sc->sc_link->flags &= ~SDEV_EJECTING;
+		if (sc_link->flags & SDEV_EJECTING) {
+			scsi_start(sc_link, SSS_STOP|SSS_LOEJ, 0);
+			sc_link->flags &= ~SDEV_EJECTING;
 		}
 
 		timeout_del(&sc->sc_timeout);
@@ -504,6 +506,7 @@ sdclose(dev_t dev, int flag, int fmt, struct proc *p)
 void
 sdstrategy(struct buf *bp)
 {
+	struct scsi_link *sc_link;
 	struct sd_softc *sc;
 	int s;
 
@@ -516,14 +519,15 @@ sdstrategy(struct buf *bp)
 		bp->b_error = ENXIO;
 		goto bad;
 	}
+	sc_link = sc->sc_link;
 
-	SC_DEBUG(sc->sc_link, SDEV_DB2, ("sdstrategy: %ld bytes @ blk %lld\n",
+	SC_DEBUG(sc_link, SDEV_DB2, ("sdstrategy: %ld bytes @ blk %lld\n",
 	    bp->b_bcount, (long long)bp->b_blkno));
 	/*
 	 * If the device has been made invalid, error out
 	 */
-	if ((sc->sc_link->flags & SDEV_MEDIA_LOADED) == 0) {
-		if (sc->sc_link->flags & SDEV_OPEN)
+	if ((sc_link->flags & SDEV_MEDIA_LOADED) == 0) {
+		if (sc_link->flags & SDEV_OPEN)
 			bp->b_error = EIO;
 		else
 			bp->b_error = ENODEV;
@@ -770,12 +774,14 @@ retry:
 void
 sdminphys(struct buf *bp)
 {
+	struct scsi_link *sc_link;
 	struct sd_softc *sc;
 	long max;
 
 	sc = sdlookup(DISKUNIT(bp->b_dev));
 	if (sc == NULL)
 		return;  /* XXX - right way to fail this? */
+	sc_link = sc->sc_link;
 
 	/*
 	 * If the device is ancient, we want to make sure that
@@ -795,7 +801,7 @@ sdminphys(struct buf *bp)
 			bp->b_bcount = max;
 	}
 
-	(*sc->sc_link->adapter->scsi_minphys)(bp, sc->sc_link);
+	(*sc_link->adapter->scsi_minphys)(bp, sc_link);
 
 	device_unref(&sc->sc_dev);
 }
@@ -819,6 +825,7 @@ sdwrite(dev_t dev, struct uio *uio, int ioflag)
 int
 sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 {
+	struct scsi_link *sc_link;
 	struct sd_softc *sc;
 	struct disklabel *lp;
 	int error = 0;
@@ -831,13 +838,14 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 		device_unref(&sc->sc_dev);
 		return (ENXIO);
 	}
+	sc_link = sc->sc_link;
 
-	SC_DEBUG(sc->sc_link, SDEV_DB2, ("sdioctl 0x%lx\n", cmd));
+	SC_DEBUG(sc_link, SDEV_DB2, ("sdioctl 0x%lx\n", cmd));
 
 	/*
 	 * If the device is not valid.. abandon ship
 	 */
-	if ((sc->sc_link->flags & SDEV_MEDIA_LOADED) == 0) {
+	if ((sc_link->flags & SDEV_MEDIA_LOADED) == 0) {
 		switch (cmd) {
 		case DIOCLOCK:
 		case DIOCEJECT:
@@ -848,7 +856,7 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 				break;
 		/* FALLTHROUGH */
 		default:
-			if ((sc->sc_link->flags & SDEV_OPEN) == 0) {
+			if ((sc_link->flags & SDEV_OPEN) == 0) {
 				error = ENODEV;
 				goto exit;
 			} else {
@@ -902,7 +910,7 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 		goto exit;
 
 	case DIOCLOCK:
-		error = scsi_prevent(sc->sc_link,
+		error = scsi_prevent(sc_link,
 		    (*(int *)addr) ? PR_PREVENT : PR_ALLOW, 0);
 		goto exit;
 
@@ -913,15 +921,15 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 		}
 		/* FALLTHROUGH */
 	case DIOCEJECT:
-		if ((sc->sc_link->flags & SDEV_REMOVABLE) == 0) {
+		if ((sc_link->flags & SDEV_REMOVABLE) == 0) {
 			error = ENOTTY;
 			goto exit;
 		}
-		sc->sc_link->flags |= SDEV_EJECTING;
+		sc_link->flags |= SDEV_EJECTING;
 		goto exit;
 
 	case DIOCINQ:
-		error = scsi_do_ioctl(sc->sc_link, cmd, addr, flag);
+		error = scsi_do_ioctl(sc_link, cmd, addr, flag);
 		if (error == ENOTTY)
 			error = sd_ioctl_inquiry(sc,
 			    (struct dk_inquiry *)addr);
@@ -942,7 +950,7 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 			error = ENOTTY;
 			goto exit;
 		}
-		error = scsi_do_ioctl(sc->sc_link, cmd, addr, flag);
+		error = scsi_do_ioctl(sc_link, cmd, addr, flag);
 	}
 
  exit:
@@ -953,21 +961,23 @@ sdioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 int
 sd_ioctl_inquiry(struct sd_softc *sc, struct dk_inquiry *di)
 {
+	struct scsi_link *sc_link;
 	struct scsi_vpd_serial *vpd;
 
 	vpd = dma_alloc(sizeof(*vpd), PR_WAITOK | PR_ZERO);
 
+	sc_link = sc->sc_link;
+
 	bzero(di, sizeof(struct dk_inquiry));
-	scsi_strvis(di->vendor, sc->sc_link->inqdata.vendor,
-	    sizeof(sc->sc_link->inqdata.vendor));
-	scsi_strvis(di->product, sc->sc_link->inqdata.product,
-	    sizeof(sc->sc_link->inqdata.product));
-	scsi_strvis(di->revision, sc->sc_link->inqdata.revision,
-	    sizeof(sc->sc_link->inqdata.revision));
+	scsi_strvis(di->vendor, sc_link->inqdata.vendor,
+	    sizeof(sc_link->inqdata.vendor));
+	scsi_strvis(di->product, sc_link->inqdata.product,
+	    sizeof(sc_link->inqdata.product));
+	scsi_strvis(di->revision, sc_link->inqdata.revision,
+	    sizeof(sc_link->inqdata.revision));
 
 	/* the serial vpd page is optional */
-	if (scsi_inquire_vpd(sc->sc_link, vpd, sizeof(*vpd),
-	    SI_PG_SERIAL, 0) == 0)
+	if (scsi_inquire_vpd(sc_link, vpd, sizeof(*vpd), SI_PG_SERIAL, 0) == 0)
 		scsi_strvis(di->serial, vpd->serial, sizeof(vpd->serial));
 	else
 		strlcpy(di->serial, "(unknown)", sizeof(vpd->serial));
@@ -979,17 +989,20 @@ sd_ioctl_inquiry(struct sd_softc *sc, struct dk_inquiry *di)
 int
 sd_ioctl_cache(struct sd_softc *sc, long cmd, struct dk_cache *dkc)
 {
+	struct scsi_link *sc_link;
 	union scsi_mode_sense_buf *buf;
 	struct page_caching_mode *mode = NULL;
 	u_int wrcache, rdcache;
 	int big;
 	int rv;
 
-	if (ISSET(sc->sc_link->flags, SDEV_UMASS))
+	sc_link = sc->sc_link;
+
+	if (ISSET(sc_link->flags, SDEV_UMASS))
 		return (EOPNOTSUPP);
 
 	/* see if the adapter has special handling */
-	rv = scsi_do_ioctl(sc->sc_link, cmd, (caddr_t)dkc, 0);
+	rv = scsi_do_ioctl(sc_link, cmd, (caddr_t)dkc, 0);
 	if (rv != ENOTTY)
 		return (rv);
 
@@ -997,7 +1010,7 @@ sd_ioctl_cache(struct sd_softc *sc, long cmd, struct dk_cache *dkc)
 	if (buf == NULL)
 		return (ENOMEM);
 
-	rv = scsi_do_mode_sense(sc->sc_link, PAGE_CACHING_MODE,
+	rv = scsi_do_mode_sense(sc_link, PAGE_CACHING_MODE,
 	    buf, (void **)&mode, NULL, NULL, NULL,
 	    sizeof(*mode) - 4, scsi_autoconf | SCSI_SILENT, &big);
 	if (rv != 0)
@@ -1032,10 +1045,10 @@ sd_ioctl_cache(struct sd_softc *sc, long cmd, struct dk_cache *dkc)
 			SET(mode->flags, PG_CACHE_FL_RCD);
 
 		if (big) {
-			rv = scsi_mode_select_big(sc->sc_link, SMS_PF,
+			rv = scsi_mode_select_big(sc_link, SMS_PF,
 			    &buf->hdr_big, scsi_autoconf | SCSI_SILENT, 20000);
 		} else {
-			rv = scsi_mode_select(sc->sc_link, SMS_PF,
+			rv = scsi_mode_select(sc_link, SMS_PF,
 			    &buf->hdr, scsi_autoconf | SCSI_SILENT, 20000);
 		}
 		break;
@@ -1053,10 +1066,13 @@ int
 sdgetdisklabel(dev_t dev, struct sd_softc *sc, struct disklabel *lp,
     int spoofonly)
 {
+	struct scsi_link *sc_link;
 	size_t len;
 	char packname[sizeof(lp->d_packname) + 1];
 	char product[17], vendor[9];
 
+	sc_link = sc->sc_link;
+
 	bzero(lp, sizeof(struct disklabel));
 
 	lp->d_secsize = sc->params.secsize;
@@ -1070,7 +1086,7 @@ sdgetdisklabel(dev_t dev, struct sd_softc *sc, struct disklabel *lp,
 	}
 
 	lp->d_type = DTYPE_SCSI;
-	if ((sc->sc_link->inqdata.device & SID_TYPE) == T_OPTICAL)
+	if ((sc_link->inqdata.device & SID_TYPE) == T_OPTICAL)
 		strncpy(lp->d_typename, "SCSI optical",
 		    sizeof(lp->d_typename));
 	else
@@ -1082,8 +1098,8 @@ sdgetdisklabel(dev_t dev, struct sd_softc *sc, struct disklabel *lp,
 	 * then leave out '<vendor> ' and use only as much of '<product>' as
 	 * does fit.
 	 */
-	viscpy(vendor, sc->sc_link->inqdata.vendor, 8);
-	viscpy(product, sc->sc_link->inqdata.product, 16);
+	viscpy(vendor, sc_link->inqdata.vendor, 8);
+	viscpy(product, sc_link->inqdata.product, 16);
 	len = snprintf(packname, sizeof(packname), "%s %s", vendor, product);
 	if (len > sizeof(lp->d_packname)) {
 		strlcpy(packname, product, sizeof(packname));
@@ -1123,8 +1139,7 @@ int
 sd_interpret_sense(struct scsi_xfer *xs)
 {
 	struct scsi_sense_data *sense = &xs->sense;
-	struct scsi_link *sc_link = xs->sc_link;
-	struct sd_softc *sc = sc_link->device_softc;
+	struct scsi_link *link = xs->sc_link;
 	u_int8_t serr = sense->error_code & SSD_ERRCODE;
 	int retval;
 
@@ -1132,7 +1147,7 @@ sd_interpret_sense(struct scsi_xfer *xs)
 	 * Let the generic code handle everything except a few categories of
 	 * LUN not ready errors on open devices.
 	 */
-	if (((sc_link->flags & SDEV_OPEN) == 0) ||
+	if (((link->flags & SDEV_OPEN) == 0) ||
 	    (serr != SSD_ERRCODE_CURRENT && serr != SSD_ERRCODE_DEFERRED) ||
 	    ((sense->flags & SSD_KEY) != SKEY_NOT_READY) ||
 	    (sense->extra_len < 6))
@@ -1143,13 +1158,13 @@ sd_interpret_sense(struct scsi_xfer *xs)
 
 	switch (ASC_ASCQ(sense)) {
 	case SENSE_NOT_READY_BECOMING_READY:
-		SC_DEBUG(sc_link, SDEV_DB1, ("becoming ready.\n"));
+		SC_DEBUG(link, SDEV_DB1, ("becoming ready.\n"));
 		retval = scsi_delay(xs, 5);
 		break;
 
 	case SENSE_NOT_READY_INIT_REQUIRED:
-		SC_DEBUG(sc_link, SDEV_DB1, ("spinning up\n"));
-		retval = scsi_start(sc->sc_link, SSS_START,
+		SC_DEBUG(link, SDEV_DB1, ("spinning up\n"));
+		retval = scsi_start(link, SSS_START,
 		    SCSI_IGNORE_ILLEGAL_REQUEST | SCSI_NOSLEEP);
 		if (retval == 0)
 			retval = ERESTART;
@@ -1157,7 +1172,7 @@ sd_interpret_sense(struct scsi_xfer *xs)
 			/* Can't issue the command. Fall back on a delay. */
 			retval = scsi_delay(xs, 5);
 		else
-			SC_DEBUG(sc_link, SDEV_DB1, ("spin up failed (%#x)\n",
+			SC_DEBUG(link, SDEV_DB1, ("spin up failed (%#x)\n",
 			    retval));
 		break;
 
@@ -1589,6 +1604,7 @@ sd_thin_params(struct sd_softc *sc, int flags)
 int
 sd_get_parms(struct sd_softc *sc, struct disk_parms *dp, int flags)
 {
+	struct scsi_link *sc_link;
 	union scsi_mode_sense_buf *buf = NULL;
 	struct page_rigid_geometry *rigid = NULL;
 	struct page_flex_geometry *flex = NULL;
@@ -1609,19 +1625,21 @@ sd_get_parms(struct sd_softc *sc, struct disk_parms *dp, int flags)
 	if (buf == NULL)
 		goto validate;
 
+	sc_link = sc->sc_link;
+
 	/*
 	 * Ask for page 0 (vendor specific) mode sense data to find
 	 * READONLY info. The only thing USB devices will ask for.
 	 */
-	err = scsi_do_mode_sense(sc->sc_link, 0, buf, (void **)&page0,
+	err = scsi_do_mode_sense(sc_link, 0, buf, (void **)&page0,
 	    NULL, NULL, NULL, 1, flags | SCSI_SILENT, &big);
 	if (err == 0) {
 		if (big && buf->hdr_big.dev_spec & SMH_DSP_WRITE_PROT)
-			SET(sc->sc_link->flags, SDEV_READONLY);
+			SET(sc_link->flags, SDEV_READONLY);
 		else if (!big && buf->hdr.dev_spec & SMH_DSP_WRITE_PROT)
-			SET(sc->sc_link->flags, SDEV_READONLY);
+			SET(sc_link->flags, SDEV_READONLY);
 		else
-			CLR(sc->sc_link->flags, SDEV_READONLY);
+			CLR(sc_link->flags, SDEV_READONLY);
 	}
 
 	/*
@@ -1629,17 +1647,17 @@ sd_get_parms(struct sd_softc *sc, struct disk_parms *dp, int flags)
 	 * don't have a meaningful geometry anyway, so just fake it if
 	 * scsi_size() worked.
 	 */
-	if ((sc->sc_link->flags & SDEV_UMASS) && (dp->disksize > 0))
+	if ((sc_link->flags & SDEV_UMASS) && (dp->disksize > 0))
 		goto validate;
 
-	switch (sc->sc_link->inqdata.device & SID_TYPE) {
+	switch (sc_link->inqdata.device & SID_TYPE) {
 	case T_OPTICAL:
 		/* No more information needed or available. */
 		break;
 
 	case T_RDIRECT:
 		/* T_RDIRECT supports only PAGE_REDUCED_GEOMETRY (6). */
-		err = scsi_do_mode_sense(sc->sc_link, PAGE_REDUCED_GEOMETRY,
+		err = scsi_do_mode_sense(sc_link, PAGE_REDUCED_GEOMETRY,
 		    buf, (void **)&reduced, NULL, NULL, &secsize,
 		    sizeof(*reduced), flags | SCSI_SILENT, NULL);
 		if (!err && reduced &&
@@ -1659,9 +1677,9 @@ sd_get_parms(struct sd_softc *sc, struct disk_parms *dp, int flags)
 		 * so accept the page. The extra bytes will be zero and RPM will
 		 * end up with the default value of 3600.
 		 */
-		if (((sc->sc_link->flags & SDEV_ATAPI) == 0) ||
-		    ((sc->sc_link->flags & SDEV_REMOVABLE) == 0))
-			err = scsi_do_mode_sense(sc->sc_link,
+		if (((sc_link->flags & SDEV_ATAPI) == 0) ||
+		    ((sc_link->flags & SDEV_REMOVABLE) == 0))
+			err = scsi_do_mode_sense(sc_link,
 			    PAGE_RIGID_GEOMETRY, buf, (void **)&rigid, NULL,
 			    NULL, &secsize, sizeof(*rigid) - 4,
 			    flags | SCSI_SILENT, NULL);
@@ -1671,7 +1689,7 @@ sd_get_parms(struct sd_softc *sc, struct disk_parms *dp, int flags)
 			if (heads * cyls > 0)
 				sectors = dp->disksize / (heads * cyls);
 		} else {
-			err = scsi_do_mode_sense(sc->sc_link,
+			err = scsi_do_mode_sense(sc_link,
 			    PAGE_FLEX_GEOMETRY, buf, (void **)&flex, NULL, NULL,
 			    &secsize, sizeof(*flex) - 4,
 			    flags | SCSI_SILENT, NULL);
@@ -1751,11 +1769,13 @@ validate:
 void
 sd_flush(struct sd_softc *sc, int flags)
 {
-	struct scsi_link *link = sc->sc_link;
+	struct scsi_link *sc_link;
 	struct scsi_xfer *xs;
 	struct scsi_synchronize_cache *cmd;
 
-	if (link->quirks & SDEV_NOSYNCCACHE)
+	sc_link = sc->sc_link;
+
+	if (sc_link->quirks & SDEV_NOSYNCCACHE)
 		return;
 
 	/*
@@ -1764,9 +1784,9 @@ sd_flush(struct sd_softc *sc, int flags)
 	 * that the command is not supported by the device.
 	 */
 
-	xs = scsi_xs_get(link, flags);
+	xs = scsi_xs_get(sc_link, flags);
 	if (xs == NULL) {
-		SC_DEBUG(link, SDEV_DB1, ("cache sync failed to get xs\n"));
+		SC_DEBUG(sc_link, SDEV_DB1, ("cache sync failed to get xs\n"));
 		return;
 	}
 
@@ -1780,7 +1800,7 @@ sd_flush(struct sd_softc *sc, int flags)
 	if (scsi_xs_sync(xs) == 0)
 		sc->flags &= ~SDF_DIRTY;
 	else
-		SC_DEBUG(link, SDEV_DB1, ("cache sync failed\n"));
+		SC_DEBUG(sc_link, SDEV_DB1, ("cache sync failed\n"));
 
 	scsi_xs_put(xs);
 }