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
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 <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Acked-by: Christoph Hellwig <[email protected]>
---
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
This helper function is useful for the nvme subsystem, and maybe
others.
Signed-off-by: Martin Wilck <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> Reviewed-by: Hannes Reinecke <[email protected]>
> Acked-by: Christoph Hellwig <[email protected]>
>
> ---
> 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