Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1693027 > unrolled thread

[PATCH v2 0/3] Improve readbility of NVME "wwid" attribute

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] Improve readbility of NVME "wwid" attribute Martin Wilck <mwilck@suse.com> - 2017-07-20 17:30 +0200
    [PATCH v2 1/3] string.h: add memcpy_and_pad() Martin Wilck <mwilck@suse.com> - 2017-07-20 17:30 +0200
    [PATCH v2 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck <mwilck@suse.com> - 2017-07-20 17:30 +0200
      Re: [PATCH v2 3/3] nvme: wwid_show: strip trailing 0-bytes Joe Perches <joe@perches.com> - 2017-07-20 18:00 +0200
    [PATCH v2 2/3] nvmet: identify controller: improve standard compliance Martin Wilck <mwilck@suse.com> - 2017-07-20 17:30 +0200

#1693027 — [PATCH v2 0/3] Improve readbility of NVME "wwid" attribute

With the current implementation, the default "fallback" WWID generation
code (if no nguid, euid etc. are defined) for Linux NVME host and target
results in the following WWID format:

nvme.0000-3163653363666438366239656630386200-4c696e75780000000000000000000000000000000000000000000000000000000000000000000000-00000002

This is not only hard to read, it poses real problems e.g. for multipath
(dm WWIDs are limited to 128 characters).

With this patch series, the WWID on a Linux host connected to a Linux target
looks like this:

nvme.0000-65613435333665653738613464363961-4c696e7578-00000001

Changes wrt v1:
 * 1/3: new, moved helper to include/linux/string.h (Christoph Hellwig)
        (you suggested kernel.h, but I think this matches string.h better)      
 * Dropped the last patch from the v1 series that would have changed valid WWIDs for
   HW NVME controllers.

Martin Wilck (3):
  string.h: add memcpy_and_pad()
  nvmet: identify controller: improve standard compliance
  nvme: wwid_show: strip trailing 0-bytes

 drivers/nvme/host/core.c        |  6 ++++--
 drivers/nvme/target/admin-cmd.c | 13 ++++++-------
 include/linux/string.h          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 9 deletions(-)

-- 
2.13.2

[toc] | [next] | [standalone]

#1693030 — [PATCH v2 1/3] string.h: add memcpy_and_pad()

This helper function is useful for the nvme subsystem, and maybe
others.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 include/linux/string.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index a467e617eeb08..0bec4151b0eb9 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -200,6 +200,7 @@ static inline const char *kbasename(const char *path)
 void fortify_panic(const char *name) __noreturn __cold;
 void __read_overflow(void) __compiletime_error("detected read beyond size of object passed as 1st parameter");
 void __read_overflow2(void) __compiletime_error("detected read beyond size of object passed as 2nd parameter");
+void __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter");
 void __write_overflow(void) __compiletime_error("detected write beyond size of object passed as 1st parameter");
 
 #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE)
@@ -395,4 +396,33 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
 
 #endif
 
+/**
+ * memcpy_and_pad - Copy one buffer to another with padding
+ * @dest: Where to copy to
+ * @dest_len: The destination buffer size
+ * @src: Where to copy from
+ * @count: The number of bytes to copy
+ * @pad: Character to use for padding if space is left in destination.
+ */
+__FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len,
+				     const void *src, size_t count, int pad)
+{
+	size_t dest_size = __builtin_object_size(dest, 0);
+	size_t src_size = __builtin_object_size(src, 0);
+
+	if (__builtin_constant_p(dest_len) && __builtin_constant_p(count)) {
+		if (dest_size < dest_len && dest_size < count)
+			__write_overflow();
+		else if (src_size < dest_len && src_size < count)
+			__read_overflow3();
+	}
+	if (dest_size < dest_len)
+		fortify_panic(__func__);
+	if (dest_len > count) {
+		memcpy(dest, src, count);
+		memset(dest + count, pad,  dest_len - count);
+	} else
+		memcpy(dest, src, dest_len);
+}
+
 #endif /* _LINUX_STRING_H_ */
-- 
2.13.2

[toc] | [prev] | [next] | [standalone]

#1693034 — [PATCH v2 3/3] nvme: wwid_show: strip trailing 0-bytes

Some broken targets (such as the current Linux target) pad
model or serial fields with 0-bytes rather than spaces. The
NVME spec disallows 0 bytes in "ASCII" fields.
Thus strip trailing 0-bytes, too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Acked-by: Christoph Hellwig <hch@lst.de>

---
 drivers/nvme/host/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index cb96f4a7ae3a9..30b87600157d7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2001,9 +2001,11 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
 	if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
 		return sprintf(buf, "eui.%8phN\n", ns->eui);
 
-	while (ctrl->serial[serial_len - 1] == ' ')
+	while (ctrl->serial[serial_len - 1] == ' ' ||
+	       ctrl->serial[serial_len - 1] == '\0')
 		serial_len--;
-	while (ctrl->model[model_len - 1] == ' ')
+	while (ctrl->model[model_len - 1] == ' ' ||
+	       ctrl->model[model_len - 1] == '\0')
 		model_len--;
 
 	return sprintf(buf, "nvme.%04x-%*phN-%*phN-%08x\n", ctrl->vid,
-- 
2.13.2

[toc] | [prev] | [next] | [standalone]

#1693052 — Re: [PATCH v2 3/3] nvme: wwid_show: strip trailing 0-bytes

On Thu, 2017-07-20 at 17:27 +0200, Martin Wilck wrote:
> Some broken targets (such as the current Linux target) pad
> model or serial fields with 0-bytes rather than spaces. The
> NVME spec disallows 0 bytes in "ASCII" fields.
> Thus strip trailing 0-bytes, too.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Acked-by: Christoph Hellwig <hch@lst.de>
> 
> ---
>  drivers/nvme/host/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
[]
> @@ -2001,9 +2001,11 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
>  	if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
>  		return sprintf(buf, "eui.%8phN\n", ns->eui);
>  
> -	while (ctrl->serial[serial_len - 1] == ' ')
> +	while (ctrl->serial[serial_len - 1] == ' ' ||
> +	       ctrl->serial[serial_len - 1] == '\0')
>  		serial_len--;
> -	while (ctrl->model[model_len - 1] == ' ')
> +	while (ctrl->model[model_len - 1] == ' ' ||
> +	       ctrl->model[model_len - 1] == '\0')
>  		model_len--;

Please add a <foo>_len > 0 to the while loops too

[toc] | [prev] | [next] | [standalone]

#1693035 — [PATCH v2 2/3] nvmet: identify controller: improve standard compliance

The NVME standard mandates that the SN, MN, and FR fields of
the Indentify Controller Data Structure be "ASCII strings".
That means that they may not contain 0-bytes, not even string
terminators.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 drivers/nvme/target/admin-cmd.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 35f930db3c02c..bd040ae32528d 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -173,6 +173,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvme_id_ctrl *id;
 	u16 status = 0;
+	const char MODEL[] = "Linux";
 
 	id = kzalloc(sizeof(*id), GFP_KERNEL);
 	if (!id) {
@@ -184,14 +185,12 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 	id->vid = 0;
 	id->ssvid = 0;
 
-	memset(id->sn, ' ', sizeof(id->sn));
-	snprintf(id->sn, sizeof(id->sn), "%llx", ctrl->serial);
+	bin2hex(id->sn, &ctrl->serial, min(sizeof(ctrl->serial),
+					   sizeof(id->sn) / 2));
 
-	memset(id->mn, ' ', sizeof(id->mn));
-	strncpy((char *)id->mn, "Linux", sizeof(id->mn));
-
-	memset(id->fr, ' ', sizeof(id->fr));
-	strncpy((char *)id->fr, UTS_RELEASE, sizeof(id->fr));
+	memcpy_and_pad(id->mn, sizeof(id->mn), MODEL, sizeof(MODEL) - 1, ' ');
+	memcpy_and_pad(id->fr, sizeof(id->fr),
+		       UTS_RELEASE, strlen(UTS_RELEASE), ' ');
 
 	id->rab = 6;
 
-- 
2.13.2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web