Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 25 Feb 2022 22:19:49 GMT
From:      "Kenneth D. Merry" <ken@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 7c79905f67fd - stable/12 - Fix non-printable characters in NVMe model and serial numbers.
Message-ID:  <202202252219.21PMJnqI016941@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by ken:

URL: https://cgit.FreeBSD.org/src/commit/?id=7c79905f67fd0f91b3f4005296b64051523251bf

commit 7c79905f67fd0f91b3f4005296b64051523251bf
Author:     Kenneth D. Merry <ken@FreeBSD.org>
AuthorDate: 2022-01-24 21:19:25 +0000
Commit:     Kenneth D. Merry <ken@FreeBSD.org>
CommitDate: 2022-02-25 22:16:47 +0000

    Fix non-printable characters in NVMe model and serial numbers.
    
    The NVMe 1.4 spec simply says that Model and Serial numbers are
    ASCII strings.  Unlike SCSI, it doesn't prohibit non-printable
    characters or say that the strings should be padded with spaces.
    
    Since 2014, we have had cam_strvis_sbuf(), which gives additional
    options for handling non-ASCII characters.  That behavior hasn't
    been available for non-sbuf consumers, so users of cam_strvis()
    were left with having octal ASCII codes inserted.
    
    So, to avoid having garbage or octal chracters in the strings, use
    cam_strvis_sbuf() to create a new function, cam_strvis_flag(), and
    re-implement cam_strvis() using cam_strvis_flag().
    
    Now, for the NVMe drives, we can use cam_strvis_flag with the
    CAM_STRVIS_FLAG_NONASCII_SPC flag.  This transforms non-printable
    characters into spaces.
    
    sys/cam/cam.c:
            Add a new function, cam_strvis_flag(), that creates an sbuf
            on the stack with the user's destination buffer, and calls
            cam_strvis_sbuf() with the given flag argument.
    
            Re-implement cam_strvis() to call cam_strvis_flag with the
            CAM_STRVIS_FLAG_NONASCII_ESC argument.  This should be the
            equivalent of the old cam_strvis() function, except for the
            overhead of creating the sbuf and calling sbuf_putc/printf.
    
    sys/cam/cam.h:
            Declaration for cam_strvis_flag.
    
    sys/cam/nvme/nvme_all.c:
            In nvme_print_ident, use the NONASCII_SPC flag with
            cam_strvis_flag().
    
    sys/cam/nvme/nvme_da.c:
            In ndaregister(), use cam_strvis_flag() with the
            NONASCII_SPC flag for the disk description and serial
            number we report to GEOM.
    
    sys/cam/nvme/nvme_xpt.c:
            In nvme_probe_done(), use cam_strvis_flag with the
            NONASCII_SPC flag when storing the drive serial number
            in the CAM EDT.
    
    Sponsored by:   Spectra Logic
    Differential Revision:  https://reviews.freebsd.org/D33973
    
    (cherry picked from commit 3090d5045a1e5663f151ef3f50f3c7db8f9a9e3c)
---
 sys/cam/cam.c           | 41 +++++++++++------------------------------
 sys/cam/cam.h           |  2 ++
 sys/cam/nvme/nvme_all.c |  9 ++++++---
 sys/cam/nvme/nvme_da.c  | 10 ++++++----
 sys/cam/nvme/nvme_xpt.c |  7 +++++--
 5 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/sys/cam/cam.c b/sys/cam/cam.c
index 372804066f50..ac8be8f451ed 100644
--- a/sys/cam/cam.c
+++ b/sys/cam/cam.c
@@ -122,38 +122,19 @@ SYSCTL_INT(_kern_cam, OID_AUTO, sort_io_queues, CTLFLAG_RWTUN,
 void
 cam_strvis(u_int8_t *dst, const u_int8_t *src, int srclen, int dstlen)
 {
+	cam_strvis_flag(dst, src, srclen, dstlen,
+	    CAM_STRVIS_FLAG_NONASCII_ESC);
+}
 
-	/* Trim leading/trailing spaces, nulls. */
-	while (srclen > 0 && src[0] == ' ')
-		src++, srclen--;
-	while (srclen > 0
-	    && (src[srclen-1] == ' ' || src[srclen-1] == '\0'))
-		srclen--;
-
-	while (srclen > 0 && dstlen > 1) {
-		u_int8_t *cur_pos = dst;
+void
+cam_strvis_flag(u_int8_t *dst, const u_int8_t *src, int srclen, int dstlen,
+		uint32_t flags)
+{
+	struct sbuf sb;
 
-		if (*src < 0x20 || *src >= 0x80) {
-			/* SCSI-II Specifies that these should never occur. */
-			/* non-printable character */
-			if (dstlen > 4) {
-				*cur_pos++ = '\\';
-				*cur_pos++ = ((*src & 0300) >> 6) + '0';
-				*cur_pos++ = ((*src & 0070) >> 3) + '0';
-				*cur_pos++ = ((*src & 0007) >> 0) + '0';
-			} else {
-				*cur_pos++ = '?';
-			}
-		} else {
-			/* normal character */
-			*cur_pos++ = *src;
-		}
-		src++;
-		srclen--;
-		dstlen -= cur_pos - dst;
-		dst = cur_pos;
-	}
-	*dst = '\0';
+	sbuf_new(&sb, dst, dstlen, SBUF_FIXEDLEN);
+	cam_strvis_sbuf(&sb, src, srclen, flags);
+	sbuf_finish(&sb);
 }
 
 void
diff --git a/sys/cam/cam.h b/sys/cam/cam.h
index 793101034859..191addabcd8d 100644
--- a/sys/cam/cam.h
+++ b/sys/cam/cam.h
@@ -378,6 +378,8 @@ caddr_t	cam_quirkmatch(caddr_t target, caddr_t quirk_table, int num_entries,
 		       int entry_size, cam_quirkmatch_t *comp_func);
 
 void	cam_strvis(u_int8_t *dst, const u_int8_t *src, int srclen, int dstlen);
+void	cam_strvis_flag(u_int8_t *dst, const u_int8_t *src, int srclen,
+			int dstlen, uint32_t flags);
 void	cam_strvis_sbuf(struct sbuf *sb, const u_int8_t *src, int srclen,
 			uint32_t flags);
 
diff --git a/sys/cam/nvme/nvme_all.c b/sys/cam/nvme/nvme_all.c
index f96c7467cc12..1c8043a013b3 100644
--- a/sys/cam/nvme/nvme_all.c
+++ b/sys/cam/nvme/nvme_all.c
@@ -92,11 +92,14 @@ nvme_print_ident(const struct nvme_controller_data *cdata,
 {
 
 	sbuf_printf(sb, "<");
-	cam_strvis_sbuf(sb, cdata->mn, sizeof(cdata->mn), 0);
+	cam_strvis_sbuf(sb, cdata->mn, sizeof(cdata->mn),
+	    CAM_STRVIS_FLAG_NONASCII_SPC);
 	sbuf_printf(sb, " ");
-	cam_strvis_sbuf(sb, cdata->fr, sizeof(cdata->fr), 0);
+	cam_strvis_sbuf(sb, cdata->fr, sizeof(cdata->fr),
+	    CAM_STRVIS_FLAG_NONASCII_SPC);
 	sbuf_printf(sb, " ");
-	cam_strvis_sbuf(sb, cdata->sn, sizeof(cdata->sn), 0);
+	cam_strvis_sbuf(sb, cdata->sn, sizeof(cdata->sn),
+	    CAM_STRVIS_FLAG_NONASCII_SPC);
 	sbuf_printf(sb, ">\n");
 }
 
diff --git a/sys/cam/nvme/nvme_da.c b/sys/cam/nvme/nvme_da.c
index 50b5ae29fc0b..7d41d1bfa8c9 100644
--- a/sys/cam/nvme/nvme_da.c
+++ b/sys/cam/nvme/nvme_da.c
@@ -882,10 +882,12 @@ ndaregister(struct cam_periph *periph, void *arg)
 	 * d_ident and d_descr are both far bigger than the length of either
 	 *  the serial or model number strings.
 	 */
-	cam_strvis(disk->d_descr, cd->mn,
-	    NVME_MODEL_NUMBER_LENGTH, sizeof(disk->d_descr));
-	cam_strvis(disk->d_ident, cd->sn,
-	    NVME_SERIAL_NUMBER_LENGTH, sizeof(disk->d_ident));
+	cam_strvis_flag(disk->d_descr, cd->mn, NVME_MODEL_NUMBER_LENGTH,
+	    sizeof(disk->d_descr), CAM_STRVIS_FLAG_NONASCII_SPC);
+
+	cam_strvis_flag(disk->d_ident, cd->sn, NVME_SERIAL_NUMBER_LENGTH,
+	    sizeof(disk->d_ident), CAM_STRVIS_FLAG_NONASCII_SPC);
+
 	disk->d_hba_vendor = cpi.hba_vendor;
 	disk->d_hba_device = cpi.hba_device;
 	disk->d_hba_subvendor = cpi.hba_subvendor;
diff --git a/sys/cam/nvme/nvme_xpt.c b/sys/cam/nvme/nvme_xpt.c
index f3cf66f1ca19..59d144b96ae3 100644
--- a/sys/cam/nvme/nvme_xpt.c
+++ b/sys/cam/nvme/nvme_xpt.c
@@ -381,8 +381,11 @@ device_fail:	if ((path->device->flags & CAM_DEV_UNCONFIGURED) == 0)
 		path->device->serial_num = (u_int8_t *)
 		    malloc(NVME_SERIAL_NUMBER_LENGTH + 1, M_CAMXPT, M_NOWAIT);
 		if (path->device->serial_num != NULL) {
-			cam_strvis(path->device->serial_num, nvme_cdata->sn,
-			    NVME_SERIAL_NUMBER_LENGTH, NVME_SERIAL_NUMBER_LENGTH + 1);
+			cam_strvis_flag(path->device->serial_num,
+			    nvme_cdata->sn, sizeof(nvme_cdata->sn),
+			    NVME_SERIAL_NUMBER_LENGTH + 1,
+			    CAM_STRVIS_FLAG_NONASCII_SPC);
+
 			path->device->serial_num_len =
 			    strlen(path->device->serial_num);
 		}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202202252219.21PMJnqI016941>