2015-06-30 18:01:38

by Chris Metcalf

[permalink] [raw]
Subject: [GIT PULL] strscpy string copy function

Linus,

Please pull the following changes for 4.2 from:

git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git strscpy

The discussion on this died down on LKML [1], so either (a) everybody loved it and
it was obviously correct, or (b) everybody lost interest. I'm optimistically
assuming (a), but willing to hear otherwise!

[1] https://lwn.net/Articles/643140/

Chris Metcalf (3):
Make asm/word-at-a-time.h available on all architectures
string: provide strscpy() and strscpy_truncate()
tile: use global strscpy() rather than private copy

arch/arc/include/asm/Kbuild | 1 +
arch/avr32/include/asm/Kbuild | 1 +
arch/blackfin/include/asm/Kbuild | 1 +
arch/c6x/include/asm/Kbuild | 1 +
arch/cris/include/asm/Kbuild | 1 +
arch/frv/include/asm/Kbuild | 1 +
arch/hexagon/include/asm/Kbuild | 1 +
arch/ia64/include/asm/Kbuild | 1 +
arch/m32r/include/asm/Kbuild | 1 +
arch/metag/include/asm/Kbuild | 1 +
arch/microblaze/include/asm/Kbuild | 1 +
arch/mips/include/asm/Kbuild | 1 +
arch/mn10300/include/asm/Kbuild | 1 +
arch/nios2/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/score/include/asm/Kbuild | 1 +
arch/tile/gxio/mpipe.c | 33 ++---------
arch/tile/include/asm/Kbuild | 1 +
arch/um/include/asm/Kbuild | 1 +
arch/unicore32/include/asm/Kbuild | 1 +
arch/xtensa/include/asm/Kbuild | 1 +
include/asm-generic/word-at-a-time.h | 80 ++++++++++++++++++++++---
include/linux/string.h | 6 ++
lib/string.c | 110 +++++++++++++++++++++++++++++++++++
25 files changed, 213 insertions(+), 37 deletions(-)

--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com


2015-07-01 16:11:52

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] strscpy string copy function

On Tue, Jun 30, 2015 at 11:01 AM, Chris Metcalf <[email protected]> wrote:
> Linus,
>
> The discussion on this died down on LKML [1], so either (a) everybody loved
> it and
> it was obviously correct, or (b) everybody lost interest. I'm
> optimistically
> assuming (a), but willing to hear otherwise!
>
> [1] https://lwn.net/Articles/643140/

Ugh. I thought we agreed to not have the odd "make it zero-sized"
thing be the default.

Let's just make something that is a sane version of strncpy/strlcpy,
not introduce yet another "str*cpy with really odd semantics for the
corner case"

Linus

2015-07-08 20:21:18

by Chris Metcalf

[permalink] [raw]
Subject: [PATCH v2 0/3] add new strscpy() API for string copy

v2: eliminate strscpy_truncate(); the single strscpy() API in v2
just tolerates a truncated string (while returning an error value)
Also, improve some of the comparison language with strlcpy/strncpy.

This patch series addresses limitations in strncpy() and strlcpy();
both the old APIs are unpleasant, as Linus nicely summarized here:

https://lkml.org/lkml/2015/4/28/570

and of course as other folks (Greg K-H and Linus again) said last year:

https://plus.google.com/+gregkroahhartman/posts/1amLbuhWbh5

The proposed new API (strscpy(), for "s"afe string copy) has an
easy-to-use API for detecting buffer overflow, doesn't walk past the
requested copy size on the source string, and isn't subject to
thread-safety attacks like the current strlcpy implementation. See
patch 2/3 for more on why strscpy() is a good thing.

To make strscpy() work more efficiently I did the minimum tweaking
necessary to allow <asm/word-at-a-time.h> to work on all architectures,
though of course individual maintainers can still make their versions
more efficient as needed.

It's likely not necessary for per-architecture implementations of
strscpy() to be written, but I stuck with the standard __HAVE_ARCH_XXX
model just for consistency with the rest of <linux/string.h>.

I tested the implementation with a simple user-space harness, so I
believe it is correct for the corner cases I could think of. In
particular I pairwise-tested all the unaligned values of source and
dest, and tested the restriction on src page-crossing at all
unaligned offsets approaching the page boundary.

The patch series is available to be pulled from

git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git strscpy

Chris Metcalf (3):
Make asm/word-at-a-time.h available on all architectures
string: provide strscpy()
tile: use global strscpy() rather than private copy

arch/arc/include/asm/Kbuild | 1 +
arch/avr32/include/asm/Kbuild | 1 +
arch/blackfin/include/asm/Kbuild | 1 +
arch/c6x/include/asm/Kbuild | 1 +
arch/cris/include/asm/Kbuild | 1 +
arch/frv/include/asm/Kbuild | 1 +
arch/hexagon/include/asm/Kbuild | 1 +
arch/ia64/include/asm/Kbuild | 1 +
arch/m32r/include/asm/Kbuild | 1 +
arch/metag/include/asm/Kbuild | 1 +
arch/microblaze/include/asm/Kbuild | 1 +
arch/mips/include/asm/Kbuild | 1 +
arch/mn10300/include/asm/Kbuild | 1 +
arch/nios2/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/score/include/asm/Kbuild | 1 +
arch/tile/gxio/mpipe.c | 33 ++------------
arch/tile/include/asm/Kbuild | 1 +
arch/um/include/asm/Kbuild | 1 +
arch/unicore32/include/asm/Kbuild | 1 +
arch/xtensa/include/asm/Kbuild | 1 +
include/asm-generic/word-at-a-time.h | 80 ++++++++++++++++++++++++++++----
include/linux/string.h | 3 ++
lib/string.c | 88 ++++++++++++++++++++++++++++++++++++
25 files changed, 188 insertions(+), 37 deletions(-)

--
2.1.2

2015-07-08 20:21:24

by Chris Metcalf

[permalink] [raw]
Subject: [PATCH v2 1/3] Make asm/word-at-a-time.h available on all architectures

Added the x86 implementation of word-at-a-time to the
generic version, which previously only supported big-endian.

Omitted the x86-specific load_unaligned_zeropad(), which in
any case is also not present for the existing BE-only
implementation of a word-at-a-time, and is only used under
CONFIG_DCACHE_WORD_ACCESS.

Added as a "generic-y" to the Kbuilds of all architectures
that didn't previously have it.

Signed-off-by: Chris Metcalf <[email protected]>
---
arch/arc/include/asm/Kbuild | 1 +
arch/avr32/include/asm/Kbuild | 1 +
arch/blackfin/include/asm/Kbuild | 1 +
arch/c6x/include/asm/Kbuild | 1 +
arch/cris/include/asm/Kbuild | 1 +
arch/frv/include/asm/Kbuild | 1 +
arch/hexagon/include/asm/Kbuild | 1 +
arch/ia64/include/asm/Kbuild | 1 +
arch/m32r/include/asm/Kbuild | 1 +
arch/metag/include/asm/Kbuild | 1 +
arch/microblaze/include/asm/Kbuild | 1 +
arch/mips/include/asm/Kbuild | 1 +
arch/mn10300/include/asm/Kbuild | 1 +
arch/nios2/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/score/include/asm/Kbuild | 1 +
arch/tile/include/asm/Kbuild | 1 +
arch/um/include/asm/Kbuild | 1 +
arch/unicore32/include/asm/Kbuild | 1 +
arch/xtensa/include/asm/Kbuild | 1 +
include/asm-generic/word-at-a-time.h | 80 ++++++++++++++++++++++++++++++++----
22 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/arch/arc/include/asm/Kbuild b/arch/arc/include/asm/Kbuild
index be0c39e76f7c..be895ef03a51 100644
--- a/arch/arc/include/asm/Kbuild
+++ b/arch/arc/include/asm/Kbuild
@@ -49,4 +49,5 @@ generic-y += types.h
generic-y += ucontext.h
generic-y += user.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/avr32/include/asm/Kbuild b/arch/avr32/include/asm/Kbuild
index 528d70d47a54..10dd6e8470bd 100644
--- a/arch/avr32/include/asm/Kbuild
+++ b/arch/avr32/include/asm/Kbuild
@@ -20,4 +20,5 @@ generic-y += sections.h
generic-y += topology.h
generic-y += trace_clock.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/blackfin/include/asm/Kbuild b/arch/blackfin/include/asm/Kbuild
index 4bd3c3cfc9ab..9705a527b7b0 100644
--- a/arch/blackfin/include/asm/Kbuild
+++ b/arch/blackfin/include/asm/Kbuild
@@ -46,4 +46,5 @@ generic-y += types.h
generic-y += ucontext.h
generic-y += unaligned.h
generic-y += user.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/c6x/include/asm/Kbuild b/arch/c6x/include/asm/Kbuild
index ae0a51f5376c..535efdb0692d 100644
--- a/arch/c6x/include/asm/Kbuild
+++ b/arch/c6x/include/asm/Kbuild
@@ -59,4 +59,5 @@ generic-y += types.h
generic-y += ucontext.h
generic-y += user.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/cris/include/asm/Kbuild b/arch/cris/include/asm/Kbuild
index 057e51859b0a..4991539a25f6 100644
--- a/arch/cris/include/asm/Kbuild
+++ b/arch/cris/include/asm/Kbuild
@@ -26,4 +26,5 @@ generic-y += sections.h
generic-y += topology.h
generic-y += trace_clock.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/frv/include/asm/Kbuild b/arch/frv/include/asm/Kbuild
index e3f81b53578e..ca34d1cf2cdf 100644
--- a/arch/frv/include/asm/Kbuild
+++ b/arch/frv/include/asm/Kbuild
@@ -7,3 +7,4 @@ generic-y += mcs_spinlock.h
generic-y += preempt.h
generic-y += scatterlist.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
diff --git a/arch/hexagon/include/asm/Kbuild b/arch/hexagon/include/asm/Kbuild
index c7a99f860b40..872920fba0a6 100644
--- a/arch/hexagon/include/asm/Kbuild
+++ b/arch/hexagon/include/asm/Kbuild
@@ -58,4 +58,5 @@ generic-y += types.h
generic-y += ucontext.h
generic-y += unaligned.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index 9b41b4bcc073..614579fa1e49 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -8,3 +8,4 @@ generic-y += preempt.h
generic-y += scatterlist.h
generic-y += trace_clock.h
generic-y += vtime.h
+generic-y += word-at-a-time.h
diff --git a/arch/m32r/include/asm/Kbuild b/arch/m32r/include/asm/Kbuild
index 2edc793372fc..36f850dd4a1e 100644
--- a/arch/m32r/include/asm/Kbuild
+++ b/arch/m32r/include/asm/Kbuild
@@ -9,3 +9,4 @@ generic-y += preempt.h
generic-y += scatterlist.h
generic-y += sections.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
diff --git a/arch/metag/include/asm/Kbuild b/arch/metag/include/asm/Kbuild
index 0bf5d525b945..a291b3b2529e 100644
--- a/arch/metag/include/asm/Kbuild
+++ b/arch/metag/include/asm/Kbuild
@@ -54,4 +54,5 @@ generic-y += ucontext.h
generic-y += unaligned.h
generic-y += user.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/microblaze/include/asm/Kbuild b/arch/microblaze/include/asm/Kbuild
index ab564a6db5c3..966f221b2d67 100644
--- a/arch/microblaze/include/asm/Kbuild
+++ b/arch/microblaze/include/asm/Kbuild
@@ -10,3 +10,4 @@ generic-y += preempt.h
generic-y += scatterlist.h
generic-y += syscalls.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
diff --git a/arch/mips/include/asm/Kbuild b/arch/mips/include/asm/Kbuild
index 526539cbc99f..c3cfb83aca1d 100644
--- a/arch/mips/include/asm/Kbuild
+++ b/arch/mips/include/asm/Kbuild
@@ -18,4 +18,5 @@ generic-y += serial.h
generic-y += trace_clock.h
generic-y += ucontext.h
generic-y += user.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/mn10300/include/asm/Kbuild b/arch/mn10300/include/asm/Kbuild
index f892d9de47d9..30a06e2d6e42 100644
--- a/arch/mn10300/include/asm/Kbuild
+++ b/arch/mn10300/include/asm/Kbuild
@@ -9,3 +9,4 @@ generic-y += preempt.h
generic-y += scatterlist.h
generic-y += sections.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
diff --git a/arch/nios2/include/asm/Kbuild b/arch/nios2/include/asm/Kbuild
index 24b3d8999ac7..bc87081c0669 100644
--- a/arch/nios2/include/asm/Kbuild
+++ b/arch/nios2/include/asm/Kbuild
@@ -61,4 +61,5 @@ generic-y += types.h
generic-y += unaligned.h
generic-y += user.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 4b87205c230c..dfc61a151890 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -9,3 +9,4 @@ generic-y += rwsem.h
generic-y += scatterlist.h
generic-y += trace_clock.h
generic-y += vtime.h
+generic-y += word-at-a-time.h
diff --git a/arch/s390/include/asm/Kbuild b/arch/s390/include/asm/Kbuild
index c631f98fd524..27c37b3d1dad 100644
--- a/arch/s390/include/asm/Kbuild
+++ b/arch/s390/include/asm/Kbuild
@@ -6,3 +6,4 @@ generic-y += mcs_spinlock.h
generic-y += preempt.h
generic-y += scatterlist.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
diff --git a/arch/score/include/asm/Kbuild b/arch/score/include/asm/Kbuild
index 83ed116d414c..16958f529aa7 100644
--- a/arch/score/include/asm/Kbuild
+++ b/arch/score/include/asm/Kbuild
@@ -13,3 +13,4 @@ generic-y += sections.h
generic-y += trace_clock.h
generic-y += xor.h
generic-y += serial.h
+generic-y += word-at-a-time.h
diff --git a/arch/tile/include/asm/Kbuild b/arch/tile/include/asm/Kbuild
index f5433e0e34e0..0804122e579a 100644
--- a/arch/tile/include/asm/Kbuild
+++ b/arch/tile/include/asm/Kbuild
@@ -39,4 +39,5 @@ generic-y += termbits.h
generic-y += termios.h
generic-y += trace_clock.h
generic-y += types.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 9176fa11d49b..ee02364d8922 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -26,4 +26,5 @@ generic-y += sections.h
generic-y += switch_to.h
generic-y += topology.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/unicore32/include/asm/Kbuild b/arch/unicore32/include/asm/Kbuild
index 3e0c19d0f4c5..f456bfa1dd54 100644
--- a/arch/unicore32/include/asm/Kbuild
+++ b/arch/unicore32/include/asm/Kbuild
@@ -62,4 +62,5 @@ generic-y += ucontext.h
generic-y += unaligned.h
generic-y += user.h
generic-y += vga.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/arch/xtensa/include/asm/Kbuild b/arch/xtensa/include/asm/Kbuild
index 86a9ab2e2ca9..6ac0a9ab3ea3 100644
--- a/arch/xtensa/include/asm/Kbuild
+++ b/arch/xtensa/include/asm/Kbuild
@@ -29,4 +29,5 @@ generic-y += statfs.h
generic-y += termios.h
generic-y += topology.h
generic-y += trace_clock.h
+generic-y += word-at-a-time.h
generic-y += xor.h
diff --git a/include/asm-generic/word-at-a-time.h b/include/asm-generic/word-at-a-time.h
index 94f9ea8abcae..011dde083f23 100644
--- a/include/asm-generic/word-at-a-time.h
+++ b/include/asm-generic/word-at-a-time.h
@@ -1,15 +1,10 @@
#ifndef _ASM_WORD_AT_A_TIME_H
#define _ASM_WORD_AT_A_TIME_H

-/*
- * This says "generic", but it's actually big-endian only.
- * Little-endian can use more efficient versions of these
- * interfaces, see for example
- * arch/x86/include/asm/word-at-a-time.h
- * for those.
- */
-
#include <linux/kernel.h>
+#include <asm/byteorder.h>
+
+#ifdef __BIG_ENDIAN

struct word_at_a_time {
const unsigned long high_bits, low_bits;
@@ -53,4 +48,73 @@ static inline bool has_zero(unsigned long val, unsigned long *data, const struct
#define zero_bytemask(mask) (~1ul << __fls(mask))
#endif

+#else
+
+/*
+ * The optimal byte mask counting is probably going to be something
+ * that is architecture-specific. If you have a reliably fast
+ * bit count instruction, that might be better than the multiply
+ * and shift, for example.
+ */
+struct word_at_a_time {
+ const unsigned long one_bits, high_bits;
+};
+
+#define WORD_AT_A_TIME_CONSTANTS { REPEAT_BYTE(0x01), REPEAT_BYTE(0x80) }
+
+#ifdef CONFIG_64BIT
+
+/*
+ * Jan Achrenius on G+: microoptimized version of
+ * the simpler "(mask & ONEBYTES) * ONEBYTES >> 56"
+ * that works for the bytemasks without having to
+ * mask them first.
+ */
+static inline long count_masked_bytes(unsigned long mask)
+{
+ return mask*0x0001020304050608ul >> 56;
+}
+
+#else /* 32-bit case */
+
+/* Carl Chatfield / Jan Achrenius G+ version for 32-bit */
+static inline long count_masked_bytes(long mask)
+{
+ /* (000000 0000ff 00ffff ffffff) -> ( 1 1 2 3 ) */
+ long a = (0x0ff0001+mask) >> 23;
+ /* Fix the 1 for 00 case */
+ return a & mask;
+}
+
+#endif
+
+/* Return nonzero if it has a zero */
+static inline unsigned long has_zero(unsigned long a, unsigned long *bits, const struct word_at_a_time *c)
+{
+ unsigned long mask = ((a - c->one_bits) & ~a) & c->high_bits;
+ *bits = mask;
+ return mask;
+}
+
+static inline unsigned long prep_zero_mask(unsigned long a, unsigned long bits, const struct word_at_a_time *c)
+{
+ return bits;
+}
+
+static inline unsigned long create_zero_mask(unsigned long bits)
+{
+ bits = (bits - 1) & ~bits;
+ return bits >> 7;
+}
+
+/* The mask we created is directly usable as a bytemask */
+#define zero_bytemask(mask) (mask)
+
+static inline unsigned long find_zero(unsigned long mask)
+{
+ return count_masked_bytes(mask);
+}
+
+#endif /* __BIG_ENDIAN */
+
#endif /* _ASM_WORD_AT_A_TIME_H */
--
2.1.2

2015-07-08 20:21:37

by Chris Metcalf

[permalink] [raw]
Subject: [PATCH v2 2/3] string: provide strscpy()

The strscpy() API is intended to be used instead of strlcpy(),
and instead of most uses of strncpy().

- Unlike strlcpy(), it doesn't read from memory beyond (src + size).

- Unlike strlcpy() or strncpy(), the API provides an easy way to check
for destination buffer overflow: an -E2BIG error return value.

- The provided implementation is robust in the face of the source
buffer being asynchronously changed during the copy, unlike the
current implementation of strlcpy().

- Unlike strncpy(), the destination buffer will be NUL-terminated
if the string in the source buffer is too long.

- Also unlike strncpy(), the destination buffer will not be updated
beyond the NUL termination, avoiding strncpy's behavior of zeroing
the entire tail end of the destination buffer. (A memset() after
the strscpy() can be used if this behavior is desired.)

- The implementation should be reasonably performant on all
platforms since it uses the asm/word-at-a-time.h API rather than
simple byte copy. Kernel-to-kernel string copy is not considered
to be performance critical in any case.

Signed-off-by: Chris Metcalf <[email protected]>
---
include/linux/string.h | 3 ++
lib/string.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index e40099e585c9..213274a0d356 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -25,6 +25,9 @@ extern char * strncpy(char *,const char *, __kernel_size_t);
#ifndef __HAVE_ARCH_STRLCPY
size_t strlcpy(char *, const char *, size_t);
#endif
+#ifndef __HAVE_ARCH_STRSCPY
+ssize_t __must_check strscpy(char *, const char *, size_t);
+#endif
#ifndef __HAVE_ARCH_STRCAT
extern char * strcat(char *, const char *);
#endif
diff --git a/lib/string.c b/lib/string.c
index a5792019193c..0ff54eb78f52 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -27,6 +27,10 @@
#include <linux/bug.h>
#include <linux/errno.h>

+#include <asm/byteorder.h>
+#include <asm/word-at-a-time.h>
+#include <asm/page.h>
+
#ifndef __HAVE_ARCH_STRNCASECMP
/**
* strncasecmp - Case insensitive, length-limited string comparison
@@ -146,6 +150,90 @@ size_t strlcpy(char *dest, const char *src, size_t size)
EXPORT_SYMBOL(strlcpy);
#endif

+#ifndef __HAVE_ARCH_STRSCPY
+/**
+ * strscpy - Copy a C-string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @count: Size of destination buffer
+ *
+ * Copy the string, or as much of it as fits, into the dest buffer.
+ * The routine returns the number of characters copied (not including
+ * the trailing NUL) or -E2BIG if the destination buffer wasn't big enough.
+ * The behavior is undefined if the string buffers overlap.
+ * If the destination buffer isn't big enough, it is NUL terminated.
+ *
+ * Preferred to strlcpy() since the API doesn't require reading memory
+ * from the src string beyond the specified "count" bytes, and since
+ * the return value is easier to error-check than strlcpy()'s.
+ * In addition, the implementation is robust to the string changing out
+ * from underneath it, unlike the current strlcpy() implementation.
+ *
+ * Preferred to strncpy() since it always returns a valid string, and
+ * doesn't unnecessarily force the tail of the destination buffer to be
+ * zeroed. If the zeroing is desired, it's likely cleaner to use strscpy()
+ * with an overflow test, then just memset() the tail of the dest buffer.
+ */
+ssize_t strscpy(char *dest, const char *src, size_t count)
+{
+ const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
+ size_t max = count;
+ long res = 0;
+
+ if (count == 0)
+ return -E2BIG;
+
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+ /*
+ * If src is unaligned, don't cross a page boundary,
+ * since we don't know if the next page is mapped.
+ */
+ if ((long)src & (sizeof(long) - 1)) {
+ size_t limit = PAGE_SIZE - ((long)src & (PAGE_SIZE - 1));
+ if (limit < max)
+ max = limit;
+ }
+#else
+ /* If src or dest is unaligned, don't do word-at-a-time. */
+ if (((long) dest | (long) src) & (sizeof(long) - 1))
+ max = 0;
+#endif
+
+ while (max >= sizeof(unsigned long)) {
+ unsigned long c, data;
+
+ c = *(unsigned long *)(src+res);
+ *(unsigned long *)(dest+res) = c;
+ if (has_zero(c, &data, &constants)) {
+ data = prep_zero_mask(c, data, &constants);
+ data = create_zero_mask(data);
+ return res + find_zero(data);
+ }
+ res += sizeof(unsigned long);
+ count -= sizeof(unsigned long);
+ max -= sizeof(unsigned long);
+ }
+
+ while (count) {
+ char c;
+
+ c = src[res];
+ dest[res] = c;
+ if (!c)
+ return res;
+ res++;
+ count--;
+ }
+
+ /* Hit buffer length without finding a NUL; force NUL-termination. */
+ if (res)
+ dest[res-1] = '\0';
+
+ return -E2BIG;
+}
+EXPORT_SYMBOL(strscpy);
+#endif
+
#ifndef __HAVE_ARCH_STRCAT
/**
* strcat - Append one %NUL-terminated string to another
--
2.1.2

2015-07-08 20:24:26

by Chris Metcalf

[permalink] [raw]
Subject: [PATCH v2 3/3] tile: use global strscpy() rather than private copy

Now that strscpy() is a standard API, remove the local copy.

Signed-off-by: Chris Metcalf <[email protected]>
---
arch/tile/gxio/mpipe.c | 33 ++++-----------------------------
1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/arch/tile/gxio/mpipe.c b/arch/tile/gxio/mpipe.c
index ee186e13dfe6..f102048d9c0e 100644
--- a/arch/tile/gxio/mpipe.c
+++ b/arch/tile/gxio/mpipe.c
@@ -19,6 +19,7 @@
#include <linux/errno.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/string.h>

#include <gxio/iorpc_globals.h>
#include <gxio/iorpc_mpipe.h>
@@ -29,32 +30,6 @@
/* HACK: Avoid pointless "shadow" warnings. */
#define link link_shadow

-/**
- * strscpy - Copy a C-string into a sized buffer, but only if it fits
- * @dest: Where to copy the string to
- * @src: Where to copy the string from
- * @size: size of destination buffer
- *
- * Use this routine to avoid copying too-long strings.
- * The routine returns the total number of bytes copied
- * (including the trailing NUL) or zero if the buffer wasn't
- * big enough. To ensure that programmers pay attention
- * to the return code, the destination has a single NUL
- * written at the front (if size is non-zero) when the
- * buffer is not big enough.
- */
-static size_t strscpy(char *dest, const char *src, size_t size)
-{
- size_t len = strnlen(src, size) + 1;
- if (len > size) {
- if (size)
- dest[0] = '\0';
- return 0;
- }
- memcpy(dest, src, len);
- return len;
-}
-
int gxio_mpipe_init(gxio_mpipe_context_t *context, unsigned int mpipe_index)
{
char file[32];
@@ -540,7 +515,7 @@ int gxio_mpipe_link_instance(const char *link_name)
if (!context)
return GXIO_ERR_NO_DEVICE;

- if (strscpy(name.name, link_name, sizeof(name.name)) == 0)
+ if (strscpy(name.name, link_name, sizeof(name.name)) < 0)
return GXIO_ERR_NO_DEVICE;

return gxio_mpipe_info_instance_aux(context, name);
@@ -559,7 +534,7 @@ int gxio_mpipe_link_enumerate_mac(int idx, char *link_name, uint8_t *link_mac)

rv = gxio_mpipe_info_enumerate_aux(context, idx, &name, &mac);
if (rv >= 0) {
- if (strscpy(link_name, name.name, sizeof(name.name)) == 0)
+ if (strscpy(link_name, name.name, sizeof(name.name)) < 0)
return GXIO_ERR_INVAL_MEMORY_SIZE;
memcpy(link_mac, mac.mac, sizeof(mac.mac));
}
@@ -576,7 +551,7 @@ int gxio_mpipe_link_open(gxio_mpipe_link_t *link,
_gxio_mpipe_link_name_t name;
int rv;

- if (strscpy(name.name, link_name, sizeof(name.name)) == 0)
+ if (strscpy(name.name, link_name, sizeof(name.name)) < 0)
return GXIO_ERR_NO_DEVICE;

rv = gxio_mpipe_link_open_aux(context, name, flags);
--
2.1.2

2015-07-08 20:54:16

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] string: provide strscpy()

Hi Chris,

On Wed, Jul 8, 2015 at 10:20 PM, Chris Metcalf <[email protected]> wrote:
> + * strscpy - Copy a C-string into a sized buffer
> + * @dest: Where to copy the string to
> + * @src: Where to copy the string from
> + * @count: Size of destination buffer
> + *
> + * Copy the string, or as much of it as fits, into the dest buffer.
> + * The routine returns the number of characters copied (not including
> + * the trailing NUL) or -E2BIG if the destination buffer wasn't big enough.
> + * The behavior is undefined if the string buffers overlap.
> + * If the destination buffer isn't big enough, it is NUL terminated.

... unless count is zero

and it's also NUL terminated if it is big enough ;-)

Perhaps
"The destination buffer is always NUL terminated, unless it's zero-sized."

> + *
> + * Preferred to strlcpy() since the API doesn't require reading memory
> + * from the src string beyond the specified "count" bytes, and since
> + * the return value is easier to error-check than strlcpy()'s.
> + * In addition, the implementation is robust to the string changing out
> + * from underneath it, unlike the current strlcpy() implementation.
> + *
> + * Preferred to strncpy() since it always returns a valid string, and
> + * doesn't unnecessarily force the tail of the destination buffer to be
> + * zeroed. If the zeroing is desired, it's likely cleaner to use strscpy()
> + * with an overflow test, then just memset() the tail of the dest buffer.
> + */
> +ssize_t strscpy(char *dest, const char *src, size_t count)
> +{
> + const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
> + size_t max = count;
> + long res = 0;
> +
> + if (count == 0)
> + return -E2BIG;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds