* [PATCH v3 0/3] Improve readbility of NVME "wwid" attribute
@ 2017-07-20 16:33 Martin Wilck
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Martin Wilck @ 2017-07-20 16:33 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
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.
Changes wrt v2:
* 3/3: Make sure no underflow occurs (Joe Perches)
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/3] string.h: add memcpy_and_pad()
2017-07-20 16:33 [PATCH v3 0/3] Improve readbility of NVME "wwid" attribute Martin Wilck
@ 2017-07-20 16:34 ` Martin Wilck
2017-07-22 3:45 ` kbuild test robot
2017-07-23 18:18 ` kbuild test robot
2017-07-20 16:34 ` [PATCH v3 2/3] nvmet: identify controller: improve standard compliance Martin Wilck
2017-07-20 16:34 ` [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck
2 siblings, 2 replies; 22+ messages in thread
From: Martin Wilck @ 2017-07-20 16:34 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/3] nvmet: identify controller: improve standard compliance
2017-07-20 16:33 [PATCH v3 0/3] Improve readbility of NVME "wwid" attribute Martin Wilck
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
@ 2017-07-20 16:34 ` Martin Wilck
2017-07-20 16:34 ` [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck
2 siblings, 0 replies; 22+ messages in thread
From: Martin Wilck @ 2017-07-20 16:34 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes
2017-07-20 16:33 [PATCH v3 0/3] Improve readbility of NVME "wwid" attribute Martin Wilck
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
2017-07-20 16:34 ` [PATCH v3 2/3] nvmet: identify controller: improve standard compliance Martin Wilck
@ 2017-07-20 16:34 ` Martin Wilck
2017-07-20 20:11 ` Keith Busch
2017-08-10 8:45 ` Christoph Hellwig
2 siblings, 2 replies; 22+ messages in thread
From: Martin Wilck @ 2017-07-20 16:34 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
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. Also make sure that we get no
underflow for pathological input.
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..9c558ab485bbc 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 (serial_len > 0 && (ctrl->serial[serial_len - 1] == ' ' ||
+ ctrl->serial[serial_len - 1] == '\0'))
serial_len--;
- while (ctrl->model[model_len - 1] == ' ')
+ while (model_len > 0 && (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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes
2017-07-20 16:34 ` [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck
@ 2017-07-20 20:11 ` Keith Busch
2017-08-10 8:45 ` Christoph Hellwig
1 sibling, 0 replies; 22+ messages in thread
From: Keith Busch @ 2017-07-20 20:11 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Sagi Grimberg, Martin Wilck,
Johannes Thumshirn, Hannes Reinecke, linux-nvme, linux-kernel,
joe
On Thu, Jul 20, 2017 at 06:34:02PM +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. Also make sure that we get no
> underflow for pathological input.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Acked-by: Christoph Hellwig <hch@lst.de>
Looks good.
Reviewed-by: Keith Busch <keith.busch@intel.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] string.h: add memcpy_and_pad()
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
@ 2017-07-22 3:45 ` kbuild test robot
2017-07-23 18:18 ` kbuild test robot
1 sibling, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2017-07-22 3:45 UTC (permalink / raw)
To: Martin Wilck
Cc: kbuild-all, Christoph Hellwig, Keith Busch, Sagi Grimberg,
Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
[-- Attachment #1: Type: text/plain, Size: 10069 bytes --]
Hi Martin,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.13-rc1 next-20170721]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Martin-Wilck/Improve-readbility-of-NVME-wwid-attribute/20170722-110309
config: x86_64-randconfig-x005-201729 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/mod_devicetable.h:11,
from scripts/mod/devicetable-offsets.c:2:
>> include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:421:2: note: in expansion of macro 'if'
if (dest_len > count) {
^~
>> include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:419:2: note: in expansion of macro 'if'
if (dest_size < dest_len)
^~
>> include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:416:8: note: in expansion of macro 'if'
else if (src_size < dest_len && src_size < count)
^~
>> include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:414:3: note: in expansion of macro 'if'
if (dest_size < dest_len && dest_size < count)
^~
>> include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:413:2: note: in expansion of macro 'if'
if (__builtin_constant_p(dest_len) && __builtin_constant_p(count)) {
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'strcpy' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:391:2: note: in expansion of macro 'if'
if (p_size == (size_t)-1 && q_size == (size_t)-1)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:381:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'kmemdup' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:379:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:370:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr_inv' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:368:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:359:2: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memchr' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:357:2: note: in expansion of macro 'if'
if (__builtin_constant_p(size) && p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:349:2: note: in expansion of macro 'if'
if (p_size < size || q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:346:3: note: in expansion of macro 'if'
if (q_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
^
include/linux/compiler.h:154:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
^~~~~~~~~~
include/linux/string.h:344:3: note: in expansion of macro 'if'
if (p_size < size)
^~
include/linux/compiler.h:162:4: warning: '______f' is static but declared in inline function 'memcmp' which is not static
______f = { \
vim +162 include/linux/compiler.h
2bcd521a Steven Rostedt 2008-11-21 148
2bcd521a Steven Rostedt 2008-11-21 149 #ifdef CONFIG_PROFILE_ALL_BRANCHES
2bcd521a Steven Rostedt 2008-11-21 150 /*
2bcd521a Steven Rostedt 2008-11-21 151 * "Define 'is'", Bill Clinton
2bcd521a Steven Rostedt 2008-11-21 152 * "Define 'if'", Steven Rostedt
2bcd521a Steven Rostedt 2008-11-21 153 */
ab3c9c68 Linus Torvalds 2009-04-07 154 #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
ab3c9c68 Linus Torvalds 2009-04-07 155 #define __trace_if(cond) \
b33c8ff4 Arnd Bergmann 2016-02-12 156 if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
2bcd521a Steven Rostedt 2008-11-21 157 ({ \
2bcd521a Steven Rostedt 2008-11-21 158 int ______r; \
2bcd521a Steven Rostedt 2008-11-21 159 static struct ftrace_branch_data \
2bcd521a Steven Rostedt 2008-11-21 160 __attribute__((__aligned__(4))) \
2bcd521a Steven Rostedt 2008-11-21 161 __attribute__((section("_ftrace_branch"))) \
2bcd521a Steven Rostedt 2008-11-21 @162 ______f = { \
2bcd521a Steven Rostedt 2008-11-21 163 .func = __func__, \
2bcd521a Steven Rostedt 2008-11-21 164 .file = __FILE__, \
2bcd521a Steven Rostedt 2008-11-21 165 .line = __LINE__, \
2bcd521a Steven Rostedt 2008-11-21 166 }; \
2bcd521a Steven Rostedt 2008-11-21 167 ______r = !!(cond); \
97e7e4f3 Witold Baryluk 2009-03-17 168 ______f.miss_hit[______r]++; \
2bcd521a Steven Rostedt 2008-11-21 169 ______r; \
2bcd521a Steven Rostedt 2008-11-21 170 }))
2bcd521a Steven Rostedt 2008-11-21 171 #endif /* CONFIG_PROFILE_ALL_BRANCHES */
2bcd521a Steven Rostedt 2008-11-21 172
:::::: The code at line 162 was first introduced by commit
:::::: 2bcd521a684cc94befbe2ce7d5b613c841b0d304 trace: profile all if conditionals
:::::: TO: Steven Rostedt <srostedt@redhat.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26326 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/3] string.h: add memcpy_and_pad()
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
2017-07-22 3:45 ` kbuild test robot
@ 2017-07-23 18:18 ` kbuild test robot
1 sibling, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2017-07-23 18:18 UTC (permalink / raw)
To: Martin Wilck
Cc: kbuild-all, Christoph Hellwig, Keith Busch, Sagi Grimberg,
Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel, joe
[-- Attachment #1: Type: text/plain, Size: 3557 bytes --]
Hi Martin,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc1 next-20170721]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Martin-Wilck/Improve-readbility-of-NVME-wwid-attribute/20170722-110309
config: x86_64-randconfig-v0-07240033 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
cc1: warnings being treated as errors
In file included from include/linux/bitmap.h:8,
from include/linux/cpumask.h:11,
from arch/x86/include/asm/cpumask.h:4,
from arch/x86/include/asm/msr.h:10,
from arch/x86/include/asm/processor.h:20,
from arch/x86/include/asm/cpufeature.h:4,
from arch/x86/include/asm/thread_info.h:52,
from include/linux/thread_info.h:37,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:80,
from include/linux/spinlock.h:50,
from include/linux/mmzone.h:7,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from include/linux/resource_ext.h:19,
from include/linux/acpi.h:26,
from drivers/gpu//drm/i915/i915_drv.c:30:
include/linux/string.h: In function 'memcpy_and_pad':
>> include/linux/string.h:413: error: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
include/linux/string.h:414: error: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
include/linux/string.h:416: error: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
include/linux/string.h:419: error: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
include/linux/string.h:421: error: '______f' is static but declared in inline function 'memcpy_and_pad' which is not static
vim +413 include/linux/string.h
398
399 /**
400 * memcpy_and_pad - Copy one buffer to another with padding
401 * @dest: Where to copy to
402 * @dest_len: The destination buffer size
403 * @src: Where to copy from
404 * @count: The number of bytes to copy
405 * @pad: Character to use for padding if space is left in destination.
406 */
407 __FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len,
408 const void *src, size_t count, int pad)
409 {
410 size_t dest_size = __builtin_object_size(dest, 0);
411 size_t src_size = __builtin_object_size(src, 0);
412
> 413 if (__builtin_constant_p(dest_len) && __builtin_constant_p(count)) {
414 if (dest_size < dest_len && dest_size < count)
415 __write_overflow();
416 else if (src_size < dest_len && src_size < count)
417 __read_overflow3();
418 }
419 if (dest_size < dest_len)
420 fortify_panic(__func__);
421 if (dest_len > count) {
422 memcpy(dest, src, count);
423 memset(dest + count, pad, dest_len - count);
424 } else
425 memcpy(dest, src, dest_len);
426 }
427
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29271 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes
2017-07-20 16:34 ` [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck
2017-07-20 20:11 ` Keith Busch
@ 2017-08-10 8:45 ` Christoph Hellwig
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
1 sibling, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2017-08-10 8:45 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg, Martin Wilck,
Johannes Thumshirn, Hannes Reinecke, linux-nvme, linux-kernel,
joe
Thanks, applied to the nvme-4.13 tree.
Note that we already merged the earlier version of your target side
changes, can you respin them against the latest 4.13-rc tree?
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side)
2017-08-10 8:45 ` Christoph Hellwig
@ 2017-08-14 20:12 ` Martin Wilck
2017-08-14 20:12 ` [PATCH v4 1/3] nvmet_execute_identify_ctrl: don't overwrite with 0-bytes Martin Wilck
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Martin Wilck @ 2017-08-14 20:12 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel
Hi Christoph,
I'm reposting the target-side of my patch rebased against 4.13-rc
as requested.
NOTE: an error has occurred while merging the previous version of my patch.
This is fixed by patch 1/3 in the series - that's an important fix for 4.13,
please push forward. 2/3 and 3/3 move the "copy_and_pad" functionality to a generic
helper, as requested. I've split this off in case the generic function meets
criticism elsewhere.
Original cover letter:
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.
Changes wrt v2:
* 3/3: Make sure no underflow occurs (Joe Perches)
Changes wrt v3:
* Rebased on 4.13-rc3.
* Dropped client-side patch which was merged in nvme-4.13 already.
* Split off bug fix (patch 1/3).
Martin Wilck (3):
nvmet_execute_identify_ctrl: don't overwrite with 0-bytes
string.h: add memcpy_and_pad()
nvmet_execute_identify_ctrl: use memcpy_and_pad()
drivers/nvme/target/admin-cmd.c | 20 +++-----------------
include/linux/string.h | 30 ++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 17 deletions(-)
--
2.14.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 1/3] nvmet_execute_identify_ctrl: don't overwrite with 0-bytes
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
@ 2017-08-14 20:12 ` Martin Wilck
2017-08-14 20:12 ` [PATCH v4 2/3] string.h: add memcpy_and_pad() Martin Wilck
` (3 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Martin Wilck @ 2017-08-14 20:12 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel
The merged version of my patch "nvmet: don't report 0-bytes in serial
number" fails to remove two lines which should have been replaced,
so that the space-padded strings are overwritten again with 0-bytes.
Fix it.
Fixes: 42de82a8b544 nvmet: don't report 0-bytes in serial number
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
drivers/nvme/target/admin-cmd.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 2d7a98ab53fbf..a53bb6635b837 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -199,12 +199,6 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
copy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1);
copy_and_pad(id->fr, sizeof(id->fr), UTS_RELEASE, strlen(UTS_RELEASE));
- 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));
-
id->rab = 6;
/*
--
2.14.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/3] string.h: add memcpy_and_pad()
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
2017-08-14 20:12 ` [PATCH v4 1/3] nvmet_execute_identify_ctrl: don't overwrite with 0-bytes Martin Wilck
@ 2017-08-14 20:12 ` Martin Wilck
2017-09-05 7:28 ` Arnd Bergmann
2017-08-14 20:12 ` [PATCH v4 3/3] nvmet_execute_identify_ctrl: use memcpy_and_pad() Martin Wilck
` (2 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: Martin Wilck @ 2017-08-14 20:12 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel
This helper function is useful for the nvme subsystem, and maybe
others.
Note: the warnings reported by the kbuild test robot for this patch
are actually generated by the use of CONFIG_PROFILE_ALL_BRANCHES
together with __FORTIFY_INLINE.
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.14.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/3] nvmet_execute_identify_ctrl: use memcpy_and_pad()
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
2017-08-14 20:12 ` [PATCH v4 1/3] nvmet_execute_identify_ctrl: don't overwrite with 0-bytes Martin Wilck
2017-08-14 20:12 ` [PATCH v4 2/3] string.h: add memcpy_and_pad() Martin Wilck
@ 2017-08-14 20:12 ` Martin Wilck
2017-08-15 9:10 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Sagi Grimberg
2017-08-16 8:12 ` Christoph Hellwig
4 siblings, 0 replies; 22+ messages in thread
From: Martin Wilck @ 2017-08-14 20:12 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch, Sagi Grimberg
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel
This changes the earlier patch "nvmet: don't report 0-bytes
in serial number" to use the memcpy_and_pad() helper introduced
in a previous patch.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
drivers/nvme/target/admin-cmd.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index a53bb6635b837..7ccea863e0ab5 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -168,15 +168,6 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
nvmet_req_complete(req, status);
}
-static void copy_and_pad(char *dst, int dst_len, const char *src, int src_len)
-{
- int len = min(src_len, dst_len);
-
- memcpy(dst, src, len);
- if (dst_len > len)
- memset(dst + len, ' ', dst_len - len);
-}
-
static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;
@@ -196,8 +187,9 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
bin2hex(id->sn, &ctrl->subsys->serial,
min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
- copy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1);
- copy_and_pad(id->fr, sizeof(id->fr), UTS_RELEASE, strlen(UTS_RELEASE));
+ 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.14.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side)
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
` (2 preceding siblings ...)
2017-08-14 20:12 ` [PATCH v4 3/3] nvmet_execute_identify_ctrl: use memcpy_and_pad() Martin Wilck
@ 2017-08-15 9:10 ` Sagi Grimberg
2017-08-16 8:12 ` Christoph Hellwig
4 siblings, 0 replies; 22+ messages in thread
From: Sagi Grimberg @ 2017-08-15 9:10 UTC (permalink / raw)
To: Martin Wilck, Christoph Hellwig, Keith Busch
Cc: Martin Wilck, Johannes Thumshirn, Hannes Reinecke, linux-nvme,
linux-kernel
For the series,
Reviewed-by: Sagi Grimberg <sagi@grimbeg.me>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side)
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
` (3 preceding siblings ...)
2017-08-15 9:10 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Sagi Grimberg
@ 2017-08-16 8:12 ` Christoph Hellwig
4 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2017-08-16 8:12 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg, Martin Wilck,
linux-kernel, Hannes Reinecke, linux-nvme, Johannes Thumshirn
Thanks, applied patch 1 to nvme-4.13, and the rest to nvme-4.14.
Btw, for future patches please use the driver name as prefix and
not a function name.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/3] string.h: add memcpy_and_pad()
2017-08-14 20:12 ` [PATCH v4 2/3] string.h: add memcpy_and_pad() Martin Wilck
@ 2017-09-05 7:28 ` Arnd Bergmann
2017-09-05 18:18 ` Martin Wilck
2017-09-05 18:23 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
0 siblings, 2 replies; 22+ messages in thread
From: Arnd Bergmann @ 2017-09-05 7:28 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg, Martin Wilck,
Johannes Thumshirn, Hannes Reinecke, linux-nvme,
Linux Kernel Mailing List
On Mon, Aug 14, 2017 at 10:12 PM, Martin Wilck <mwilck@suse.com> wrote:
> This helper function is useful for the nvme subsystem, and maybe
> others.
>
> Note: the warnings reported by the kbuild test robot for this patch
> are actually generated by the use of CONFIG_PROFILE_ALL_BRANCHES
> together with __FORTIFY_INLINE.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> #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)
> +{
This is causing compile-time warnings for me:
In file included from /git/arm-soc/arch/x86/include/asm/string.h:2:0,
from /git/arm-soc/include/linux/string.h:18,
from /git/arm-soc/arch/x86/include/asm/page_32.h:34,
from /git/arm-soc/arch/x86/include/asm/page.h:13,
from /git/arm-soc/arch/x86/include/asm/thread_info.h:11,
from /git/arm-soc/include/linux/thread_info.h:37,
from /git/arm-soc/arch/x86/include/asm/preempt.h:6,
from /git/arm-soc/include/linux/preempt.h:80,
from /git/arm-soc/include/linux/spinlock.h:50,
from /git/arm-soc/include/linux/seqlock.h:35,
from /git/arm-soc/include/linux/time.h:5,
from /git/arm-soc/include/linux/stat.h:18,
from /git/arm-soc/include/linux/module.h:10,
from /git/arm-soc/drivers/md/dm-integrity.c:9:
/git/arm-soc/arch/x86/include/asm/string_32.h:196:25: error:
'__memcpy' is static but used in inline function 'memcpy_and_pad'
which is not static [-Werror]
#define memcpy(t, f, n) __memcpy((t), (f), (n))
^~~~~~~~
/git/arm-soc/include/linux/string.h:466:3: note: in expansion of macro 'memcpy'
^
/git/arm-soc/arch/x86/include/asm/string_32.h:196:25: error:
'__memcpy' is static but used in inline function 'memcpy_and_pad'
which is not static [-Werror]
#define memcpy(t, f, n) __memcpy((t), (f), (n))
^~~~~~~~
The problem is the use of __FORTIFY_INLINE outside of the #ifdef
section above it.
I used an ugly local workaround, duplicating the function with a
'static inline' variant
in an #else block. Alternatively we could add an extern version in
lib/string.c for the
non-fortified case.
Arnd
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/3] string.h: add memcpy_and_pad()
2017-09-05 7:28 ` Arnd Bergmann
@ 2017-09-05 18:18 ` Martin Wilck
2017-09-05 18:23 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
1 sibling, 0 replies; 22+ messages in thread
From: Martin Wilck @ 2017-09-05 18:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg,
Johannes Thumshirn, Hannes Reinecke, linux-nvme,
Linux Kernel Mailing List
On Tue, 2017-09-05 at 09:28 +0200, Arnd Bergmann wrote:
>
> > +/**
> > + * 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)
> > +{
>
> This is causing compile-time warnings for me:
>
> In file included from /git/arm-soc/arch/x86/include/asm/string.h:2:0,
> from /git/arm-soc/include/linux/string.h:18,
> from /git/arm-soc/arch/x86/include/asm/page_32.h:34,
> from /git/arm-soc/arch/x86/include/asm/page.h:13,
> from /git/arm-
> soc/arch/x86/include/asm/thread_info.h:11,
> from /git/arm-soc/include/linux/thread_info.h:37,
> from /git/arm-soc/arch/x86/include/asm/preempt.h:6,
> from /git/arm-soc/include/linux/preempt.h:80,
> from /git/arm-soc/include/linux/spinlock.h:50,
> from /git/arm-soc/include/linux/seqlock.h:35,
> from /git/arm-soc/include/linux/time.h:5,
> from /git/arm-soc/include/linux/stat.h:18,
> from /git/arm-soc/include/linux/module.h:10,
> from /git/arm-soc/drivers/md/dm-integrity.c:9:
> /git/arm-soc/arch/x86/include/asm/string_32.h:196:25: error:
> '__memcpy' is static but used in inline function 'memcpy_and_pad'
> which is not static [-Werror]
> #define memcpy(t, f, n) __memcpy((t), (f), (n))
> ^~~~~~~~
> /git/arm-soc/include/linux/string.h:466:3: note: in expansion of
> macro 'memcpy'
>
> ^
> /git/arm-soc/arch/x86/include/asm/string_32.h:196:25: error:
> '__memcpy' is static but used in inline function 'memcpy_and_pad'
> which is not static [-Werror]
> #define memcpy(t, f, n) __memcpy((t), (f), (n))
> ^~~~~~~~
>
> The problem is the use of __FORTIFY_INLINE outside of the #ifdef
> section above it.
> I used an ugly local workaround, duplicating the function with a
> 'static inline' variant
> in an #else block. Alternatively we could add an extern version in
> lib/string.c for the
> non-fortified case.
I'm sorry. It seems that I messed the code up by trying to do it right.
I suggest to simply drop the fortification code from this function,
which is not a "common str/mem function" anyway. Please tell me if
that'd be ok for you.
I'll send a patch in a follow-up email.
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] string.h: un-fortify memcpy_and_pad
2017-09-05 7:28 ` Arnd Bergmann
2017-09-05 18:18 ` Martin Wilck
@ 2017-09-05 18:23 ` Martin Wilck
2017-09-05 19:26 ` Arnd Bergmann
1 sibling, 1 reply; 22+ messages in thread
From: Martin Wilck @ 2017-09-05 18:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: arndbergmann, Christoph Hellwig, Keith Busch, Sagi Grimberg,
Johannes Thumshirn, Hannes Reinecke, linux-nvme, linux-kernel
The way I'd implemented the new helper memcpy_and_pad with
__FORTIFY_INLINE caused compiler warnings for certain kernel
configurations.
This helper is only used in a single place at this time, and thus
doesn't benefit much from fortification. So simplify the code
by dropping fortification support for now.
Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
include/linux/string.h | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index 0bec4151b0eb9..0495cd3c81689 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -404,20 +404,9 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
* @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)
+static 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);
--
2.14.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] string.h: un-fortify memcpy_and_pad
2017-09-05 18:23 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
@ 2017-09-05 19:26 ` Arnd Bergmann
2017-09-06 13:02 ` Arnd Bergmann
0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2017-09-05 19:26 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg,
Johannes Thumshirn, Hannes Reinecke, linux-nvme,
Linux Kernel Mailing List
On Tue, Sep 5, 2017 at 8:23 PM, Martin Wilck <mwilck@suse.com> wrote:
> The way I'd implemented the new helper memcpy_and_pad with
> __FORTIFY_INLINE caused compiler warnings for certain kernel
> configurations.
>
> This helper is only used in a single place at this time, and thus
> doesn't benefit much from fortification. So simplify the code
> by dropping fortification support for now.
>
> Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Looks good to me,
Acked-by: Arnd Bergmann <arnd@arndb.de>
I've added this to my randconfig testing tree, if you don't hear anything
from me by tomorrow, you can assume that it caused no other failures.
Arnd
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] string.h: un-fortify memcpy_and_pad
2017-09-05 19:26 ` Arnd Bergmann
@ 2017-09-06 13:02 ` Arnd Bergmann
0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2017-09-06 13:02 UTC (permalink / raw)
To: Martin Wilck
Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg,
Johannes Thumshirn, Hannes Reinecke, linux-nvme,
Linux Kernel Mailing List
On Tue, Sep 5, 2017 at 9:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Sep 5, 2017 at 8:23 PM, Martin Wilck <mwilck@suse.com> wrote:
>> The way I'd implemented the new helper memcpy_and_pad with
>> __FORTIFY_INLINE caused compiler warnings for certain kernel
>> configurations.
>>
>> This helper is only used in a single place at this time, and thus
>> doesn't benefit much from fortification. So simplify the code
>> by dropping fortification support for now.
>>
>> Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
>> Signed-off-by: Martin Wilck <mwilck@suse.com>
>
> Looks good to me,
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> I've added this to my randconfig testing tree, if you don't hear anything
> from me by tomorrow, you can assume that it caused no other failures.
build-tested successfully.
Tested-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] string.h: un-fortify memcpy_and_pad
2017-09-11 9:44 ` Martin Wilck
@ 2017-09-11 11:57 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2017-09-11 11:57 UTC (permalink / raw)
To: Martin Wilck
Cc: Stephen Rothwell, Christoph Hellwig, Sagi Grimberg, Kees Cook,
linux-kernel, linux-next, Jens Axboe, Andrew Morton, linux-nvme,
jthumshirn, arnd
> Hello Stephen and Christoph,
>
> my broken patch 01f33c336e2d is in Linus' tree and causing compiler
> warnings there. Could you please take care that this fix is pulled in
> on top of it? Or should I take another action myself?
string.h is a bit of a maintainers no mans land. Given that the
problematic commit came in through the nvme tree I've applied your
patch and will sned it to Jens with the next batch of nvme updates
tonight.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] string.h: un-fortify memcpy_and_pad
2017-09-06 12:36 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
@ 2017-09-11 9:44 ` Martin Wilck
2017-09-11 11:57 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Martin Wilck @ 2017-09-11 9:44 UTC (permalink / raw)
To: Stephen Rothwell, Christoph Hellwig
Cc: Sagi Grimberg, Kees Cook, linux-kernel, linux-next, Jens Axboe,
Andrew Morton, linux-nvme, jthumshirn, arnd
On Wed, 2017-09-06 at 14:36 +0200, Martin Wilck wrote:
> The way I'd implemented the new helper memcpy_and_pad with
> __FORTIFY_INLINE caused compiler warnings for certain kernel
> configurations.
>
> This helper is only used in a single place at this time, and thus
> doesn't benefit much from fortification. So simplify the code
> by dropping fortification support for now.
>
> Fixes: 01f33c336e2d "string.h: add memcpy_and_pad()"
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> ---
> include/linux/string.h | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)
Hello Stephen and Christoph,
my broken patch 01f33c336e2d is in Linus' tree and causing compiler
warnings there. Could you please take care that this fix is pulled in
on top of it? Or should I take another action myself?
Thanks,
Martin
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index e1eeb0a8a9693..54d21783e18dd 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -434,20 +434,9 @@ __FORTIFY_INLINE char *strcpy(char *p, const
> char *q)
> * @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)
> +static 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);
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] string.h: un-fortify memcpy_and_pad
2017-08-31 8:21 linux-next: build failure after merge of the akpm-current tree Stephen Rothwell
@ 2017-09-06 12:36 ` Martin Wilck
2017-09-11 9:44 ` Martin Wilck
0 siblings, 1 reply; 22+ messages in thread
From: Martin Wilck @ 2017-09-06 12:36 UTC (permalink / raw)
To: Stephen Rothwell, Christoph Hellwig, Jens Axboe, Andrew Morton
Cc: Sagi Grimberg, Kees Cook, linux-kernel, linux-next, Martin Wilck
The way I'd implemented the new helper memcpy_and_pad with
__FORTIFY_INLINE caused compiler warnings for certain kernel
configurations.
This helper is only used in a single place at this time, and thus
doesn't benefit much from fortification. So simplify the code
by dropping fortification support for now.
Fixes: 01f33c336e2d "string.h: add memcpy_and_pad()"
Signed-off-by: Martin Wilck <mwilck@suse.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/string.h | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index e1eeb0a8a9693..54d21783e18dd 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -434,20 +434,9 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
* @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)
+static 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);
--
2.14.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-09-11 11:57 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-20 16:33 [PATCH v3 0/3] Improve readbility of NVME "wwid" attribute Martin Wilck
2017-07-20 16:34 ` [PATCH v3 1/3] string.h: add memcpy_and_pad() Martin Wilck
2017-07-22 3:45 ` kbuild test robot
2017-07-23 18:18 ` kbuild test robot
2017-07-20 16:34 ` [PATCH v3 2/3] nvmet: identify controller: improve standard compliance Martin Wilck
2017-07-20 16:34 ` [PATCH v3 3/3] nvme: wwid_show: strip trailing 0-bytes Martin Wilck
2017-07-20 20:11 ` Keith Busch
2017-08-10 8:45 ` Christoph Hellwig
2017-08-14 20:12 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Martin Wilck
2017-08-14 20:12 ` [PATCH v4 1/3] nvmet_execute_identify_ctrl: don't overwrite with 0-bytes Martin Wilck
2017-08-14 20:12 ` [PATCH v4 2/3] string.h: add memcpy_and_pad() Martin Wilck
2017-09-05 7:28 ` Arnd Bergmann
2017-09-05 18:18 ` Martin Wilck
2017-09-05 18:23 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
2017-09-05 19:26 ` Arnd Bergmann
2017-09-06 13:02 ` Arnd Bergmann
2017-08-14 20:12 ` [PATCH v4 3/3] nvmet_execute_identify_ctrl: use memcpy_and_pad() Martin Wilck
2017-08-15 9:10 ` [PATCH v4 0/3] Improve readbility of NVME "wwid" attribute (target side) Sagi Grimberg
2017-08-16 8:12 ` Christoph Hellwig
2017-08-31 8:21 linux-next: build failure after merge of the akpm-current tree Stephen Rothwell
2017-09-06 12:36 ` [PATCH] string.h: un-fortify memcpy_and_pad Martin Wilck
2017-09-11 9:44 ` Martin Wilck
2017-09-11 11:57 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).