TL;DR -- don't configure devices that return insufficient INQUIRY data.

Treat INQUIRY data with fewer than SID_SCSI2_HDRLEN bytes as invalid.

Use only INQUIRY data returned by the device.

Get all available INQUIRY data (up to sizeof(struct scsi_inquiry_data))
even when SCSIDEBUG is not set.

Tweak returned INQUIRY data so additional_length field does not point
past end of returned data when available data is greater than
sizeof(struct scsi_inquiry_data).

Missing dmafree() spotted by gnezdo@. ok jmatthew@.

2 files changed, 41 insertions, 29 deletions

diff --git a/sys/scsi/scsi_base.c b/sys/scsi/scsi_base.c
index dcd9f0e5544..2ba6a702fbc 100644
--- a/sys/scsi/scsi_base.c
+++ b/sys/scsi/scsi_base.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: scsi_base.c,v 1.277 2020/10/14 23:40:33 krw Exp $	*/
+/*	$OpenBSD: scsi_base.c,v 1.278 2020/11/19 13:45:15 krw Exp $	*/
 /*	$NetBSD: scsi_base.c,v 1.43 1997/04/02 02:29:36 mycroft Exp $	*/
 
 /*
@@ -841,7 +841,7 @@ scsi_inquire(struct scsi_link *link, struct scsi_inquiry_data *inqbuf,
 {
 	struct scsi_xfer	*xs;
 	size_t			 bytes;
-	int			 error;
+	int			 avail, retries, error, received;
 
 	/*
 	 * Start by asking for only the basic 36 bytes of SCSI2 inquiry
@@ -849,42 +849,45 @@ scsi_inquire(struct scsi_link *link, struct scsi_inquiry_data *inqbuf,
 	 * supply more.
 	 */
 	bytes = SID_SCSI2_HDRLEN + SID_SCSI2_ALEN;
+	retries = 0;
 
-#ifdef SCSIDEBUG
 again:
-#endif /* SCSIDEBUG */
 	xs = scsi_xs_get(link, flags);
 	if (xs == NULL)
 		return EBUSY;
 
+	if (bytes > sizeof(*inqbuf))
+		bytes = sizeof(*inqbuf);
 	scsi_init_inquiry(xs, 0, 0, inqbuf, bytes);
 
-	bzero(inqbuf, sizeof(*inqbuf));
-	memset(&inqbuf->vendor, ' ', sizeof inqbuf->vendor);
-	memset(&inqbuf->product, ' ', sizeof inqbuf->product);
-	memset(&inqbuf->revision, ' ', sizeof inqbuf->revision);
-	memset(&inqbuf->extra, ' ', sizeof inqbuf->extra);
-
 	error = scsi_xs_sync(xs);
-
+	received = xs->datalen - xs->resid;
 	scsi_xs_put(xs);
 
-#ifdef SCSIDEBUG
-	sc_print_addr(link);
-	if (bytes > SID_SCSI2_HDRLEN + inqbuf->additional_length)
-		bytes = SID_SCSI2_HDRLEN + inqbuf->additional_length;
-	printf("got %zu of %u bytes of inquiry data:\n",
-	    bytes, SID_SCSI2_HDRLEN + inqbuf->additional_length);
-	scsi_show_mem((u_char *)inqbuf, bytes);
-	if (bytes == SID_SCSI2_HDRLEN + SID_SCSI2_ALEN && bytes <
-	    SID_SCSI2_HDRLEN + inqbuf->additional_length) {
-		bytes = SID_SCSI2_HDRLEN + inqbuf->additional_length;
-		if (bytes > sizeof(*inqbuf))
-			bytes = sizeof(*inqbuf);
+	if (error != 0)
+		return error;
+	if (received < SID_SCSI2_HDRLEN)
+		return EINVAL;
+
+	avail = SID_SCSI2_HDRLEN + inqbuf->additional_length;
+
+	if (received < avail && retries == 0) {
+		retries++;
+		bytes = avail;
 		goto again;
 	}
+
+#ifdef SCSIDEBUG
+	sc_print_addr(link);
+	printf("got %d of %d bytes of inquiry data:\n", received,
+	    avail);
+	scsi_show_mem((u_char *)inqbuf, received);
 #endif /* SCSIDEBUG */
-	return error;
+
+	if (avail > received)
+		inqbuf->additional_length = received - SID_SCSI2_HDRLEN;
+
+	return 0;
 }
 
 /*
diff --git a/sys/scsi/scsiconf.c b/sys/scsi/scsiconf.c
index 2758a61b1b0..baa4dfdcdd7 100644
--- a/sys/scsi/scsiconf.c
+++ b/sys/scsi/scsiconf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: scsiconf.c,v 1.236 2020/08/18 16:30:38 krw Exp $	*/
+/*	$OpenBSD: scsiconf.c,v 1.237 2020/11/19 13:45:15 krw Exp $	*/
 /*	$NetBSD: scsiconf.c,v 1.57 1996/05/02 01:09:01 neil Exp $	*/
 
 /*
@@ -488,7 +488,7 @@ scsi_probe_link(struct scsibus_softc *sb, int target, int lun, int dumbscan)
 	struct scsi_inquiry_data		*inqbuf, *usbinqbuf;
 	struct scsi_link			*link, *link0;
 	struct cfdata				*cf;
-	int					 priority, rslt = 0;
+	int					 inqbytes, priority, rslt = 0;
 	u_int16_t				 devquirks;
 
 	/* Skip this slot if it is already attached and try the next LUN. */
@@ -587,18 +587,27 @@ scsi_probe_link(struct scsibus_softc *sb, int target, int lun, int dumbscan)
 	}
 
 	rslt = scsi_inquire(link, inqbuf, scsi_autoconf | SCSI_SILENT);
-	memcpy(&link->inqdata, inqbuf, sizeof(link->inqdata));
-	dma_free(inqbuf, sizeof(*inqbuf));
-
 	if (rslt != 0) {
 		if (lun == 0) {
 			SC_DEBUG(link, SDEV_DB2, ("Bad LUN 0. inquiry rslt = "
 			    "%i\n", rslt));
 			rslt = EINVAL;
 		}
+		dma_free(inqbuf, sizeof(*inqbuf));
 		goto bad;
 	}
+	inqbytes = SID_SCSI2_HDRLEN + inqbuf->additional_length;
+	memcpy(&link->inqdata, inqbuf, inqbytes);
+	dma_free(inqbuf, sizeof(*inqbuf));
 	inqbuf = &link->inqdata;
+	if (inqbytes < offsetof(struct scsi_inquiry_data, vendor))
+		memset(inqbuf->vendor, ' ', sizeof(inqbuf->vendor));
+	if (inqbytes < offsetof(struct scsi_inquiry_data, product))
+		memset(inqbuf->product, ' ', sizeof(inqbuf->product));
+	if (inqbytes < offsetof(struct scsi_inquiry_data, revision))
+		memset(inqbuf->revision, ' ', sizeof(inqbuf->revision));
+	if (inqbytes < offsetof(struct scsi_inquiry_data, extra))
+		memset(inqbuf->extra, ' ', sizeof(inqbuf->extra));
 
 	switch (inqbuf->device & SID_QUAL) {
 	case SID_QUAL_RSVD: