Skip to content

Commit 30c4465

Browse files
committed
Merge branch 'strscpy' of git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile
Pull strscpy string copy function implementation from Chris Metcalf. Chris sent this during the merge window, but I waffled back and forth on the pull request, which is why it's going in only now. The new "strscpy()" function is definitely easier to use and more secure than either strncpy() or strlcpy(), both of which are horrible nasty interfaces that have serious and irredeemable problems. strncpy() has a useless return value, and doesn't NUL-terminate an overlong result. To make matters worse, it pads a short result with zeroes, which is a performance disaster if you have big buffers. strlcpy(), by contrast, is a mis-designed "fix" for strlcpy(), lacking the insane NUL padding, but having a differently broken return value which returns the original length of the source string. Which means that it will read characters past the count from the source buffer, and you have to trust the source to be properly terminated. It also makes error handling fragile, since the test for overflow is unnecessarily subtle. strscpy() avoids both these problems, guaranteeing the NUL termination (but not excessive padding) if the destination size wasn't zero, and making the overflow condition very obvious by returning -E2BIG. It also doesn't read past the size of the source, and can thus be used for untrusted source data too. So why did I waffle about this for so long? Every time we introduce a new-and-improved interface, people start doing these interminable series of trivial conversion patches. And every time that happens, somebody does some silly mistake, and the conversion patch to the improved interface actually makes things worse. Because the patch is mindnumbing and trivial, nobody has the attention span to look at it carefully, and it's usually done over large swatches of source code which means that not every conversion gets tested. So I'm pulling the strscpy() support because it *is* a better interface. But I will refuse to pull mindless conversion patches. Use this in places where it makes sense, but don't do trivial patches to fix things that aren't actually known to be broken. * 'strscpy' of git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile: tile: use global strscpy() rather than private copy string: provide strscpy() Make asm/word-at-a-time.h available on all architectures
2 parents 15ecf9a + 30059d4 commit 30c4465

File tree

25 files changed

+188
-37
lines changed

25 files changed

+188
-37
lines changed

arch/arc/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,5 @@ generic-y += types.h
4848
generic-y += ucontext.h
4949
generic-y += user.h
5050
generic-y += vga.h
51+
generic-y += word-at-a-time.h
5152
generic-y += xor.h

arch/avr32/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ generic-y += sections.h
2020
generic-y += topology.h
2121
generic-y += trace_clock.h
2222
generic-y += vga.h
23+
generic-y += word-at-a-time.h
2324
generic-y += xor.h

arch/blackfin/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,5 @@ generic-y += types.h
4646
generic-y += ucontext.h
4747
generic-y += unaligned.h
4848
generic-y += user.h
49+
generic-y += word-at-a-time.h
4950
generic-y += xor.h

arch/c6x/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,5 @@ generic-y += types.h
5959
generic-y += ucontext.h
6060
generic-y += user.h
6161
generic-y += vga.h
62+
generic-y += word-at-a-time.h
6263
generic-y += xor.h

arch/cris/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,5 @@ generic-y += topology.h
4343
generic-y += trace_clock.h
4444
generic-y += types.h
4545
generic-y += vga.h
46+
generic-y += word-at-a-time.h
4647
generic-y += xor.h

arch/frv/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ generic-y += mcs_spinlock.h
77
generic-y += mm-arch-hooks.h
88
generic-y += preempt.h
99
generic-y += trace_clock.h
10+
generic-y += word-at-a-time.h

arch/hexagon/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,5 @@ generic-y += types.h
5858
generic-y += ucontext.h
5959
generic-y += unaligned.h
6060
generic-y += vga.h
61+
generic-y += word-at-a-time.h
6162
generic-y += xor.h

arch/ia64/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ generic-y += mm-arch-hooks.h
88
generic-y += preempt.h
99
generic-y += trace_clock.h
1010
generic-y += vtime.h
11+
generic-y += word-at-a-time.h

arch/m32r/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ generic-y += module.h
99
generic-y += preempt.h
1010
generic-y += sections.h
1111
generic-y += trace_clock.h
12+
generic-y += word-at-a-time.h

arch/metag/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,5 @@ generic-y += ucontext.h
5454
generic-y += unaligned.h
5555
generic-y += user.h
5656
generic-y += vga.h
57+
generic-y += word-at-a-time.h
5758
generic-y += xor.h

arch/microblaze/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ generic-y += mm-arch-hooks.h
1010
generic-y += preempt.h
1111
generic-y += syscalls.h
1212
generic-y += trace_clock.h
13+
generic-y += word-at-a-time.h

arch/mips/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ generic-y += segment.h
1717
generic-y += serial.h
1818
generic-y += trace_clock.h
1919
generic-y += user.h
20+
generic-y += word-at-a-time.h
2021
generic-y += xor.h

arch/mn10300/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ generic-y += mm-arch-hooks.h
99
generic-y += preempt.h
1010
generic-y += sections.h
1111
generic-y += trace_clock.h
12+
generic-y += word-at-a-time.h

arch/nios2/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,5 @@ generic-y += types.h
6161
generic-y += unaligned.h
6262
generic-y += user.h
6363
generic-y += vga.h
64+
generic-y += word-at-a-time.h
6465
generic-y += xor.h

arch/powerpc/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ generic-y += mcs_spinlock.h
77
generic-y += preempt.h
88
generic-y += rwsem.h
99
generic-y += vtime.h
10+
generic-y += word-at-a-time.h

arch/s390/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ generic-y += mcs_spinlock.h
66
generic-y += mm-arch-hooks.h
77
generic-y += preempt.h
88
generic-y += trace_clock.h
9+
generic-y += word-at-a-time.h

arch/score/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ generic-y += sections.h
1313
generic-y += trace_clock.h
1414
generic-y += xor.h
1515
generic-y += serial.h
16+
generic-y += word-at-a-time.h

arch/tile/gxio/mpipe.c

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <linux/errno.h>
2020
#include <linux/io.h>
2121
#include <linux/module.h>
22+
#include <linux/string.h>
2223

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

32-
/**
33-
* strscpy - Copy a C-string into a sized buffer, but only if it fits
34-
* @dest: Where to copy the string to
35-
* @src: Where to copy the string from
36-
* @size: size of destination buffer
37-
*
38-
* Use this routine to avoid copying too-long strings.
39-
* The routine returns the total number of bytes copied
40-
* (including the trailing NUL) or zero if the buffer wasn't
41-
* big enough. To ensure that programmers pay attention
42-
* to the return code, the destination has a single NUL
43-
* written at the front (if size is non-zero) when the
44-
* buffer is not big enough.
45-
*/
46-
static size_t strscpy(char *dest, const char *src, size_t size)
47-
{
48-
size_t len = strnlen(src, size) + 1;
49-
if (len > size) {
50-
if (size)
51-
dest[0] = '\0';
52-
return 0;
53-
}
54-
memcpy(dest, src, len);
55-
return len;
56-
}
57-
5833
int gxio_mpipe_init(gxio_mpipe_context_t *context, unsigned int mpipe_index)
5934
{
6035
char file[32];
@@ -540,7 +515,7 @@ int gxio_mpipe_link_instance(const char *link_name)
540515
if (!context)
541516
return GXIO_ERR_NO_DEVICE;
542517

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

546521
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)
559534

560535
rv = gxio_mpipe_info_enumerate_aux(context, idx, &name, &mac);
561536
if (rv >= 0) {
562-
if (strscpy(link_name, name.name, sizeof(name.name)) == 0)
537+
if (strscpy(link_name, name.name, sizeof(name.name)) < 0)
563538
return GXIO_ERR_INVAL_MEMORY_SIZE;
564539
memcpy(link_mac, mac.mac, sizeof(mac.mac));
565540
}
@@ -576,7 +551,7 @@ int gxio_mpipe_link_open(gxio_mpipe_link_t *link,
576551
_gxio_mpipe_link_name_t name;
577552
int rv;
578553

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

582557
rv = gxio_mpipe_link_open_aux(context, name, flags);

arch/tile/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,5 @@ generic-y += termbits.h
4040
generic-y += termios.h
4141
generic-y += trace_clock.h
4242
generic-y += types.h
43+
generic-y += word-at-a-time.h
4344
generic-y += xor.h

arch/um/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@ generic-y += preempt.h
2525
generic-y += switch_to.h
2626
generic-y += topology.h
2727
generic-y += trace_clock.h
28+
generic-y += word-at-a-time.h
2829
generic-y += xor.h

arch/unicore32/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,5 @@ generic-y += ucontext.h
6262
generic-y += unaligned.h
6363
generic-y += user.h
6464
generic-y += vga.h
65+
generic-y += word-at-a-time.h
6566
generic-y += xor.h

arch/xtensa/include/asm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ generic-y += statfs.h
2828
generic-y += termios.h
2929
generic-y += topology.h
3030
generic-y += trace_clock.h
31+
generic-y += word-at-a-time.h
3132
generic-y += xor.h

include/asm-generic/word-at-a-time.h

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
11
#ifndef _ASM_WORD_AT_A_TIME_H
22
#define _ASM_WORD_AT_A_TIME_H
33

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

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

51+
#else
52+
53+
/*
54+
* The optimal byte mask counting is probably going to be something
55+
* that is architecture-specific. If you have a reliably fast
56+
* bit count instruction, that might be better than the multiply
57+
* and shift, for example.
58+
*/
59+
struct word_at_a_time {
60+
const unsigned long one_bits, high_bits;
61+
};
62+
63+
#define WORD_AT_A_TIME_CONSTANTS { REPEAT_BYTE(0x01), REPEAT_BYTE(0x80) }
64+
65+
#ifdef CONFIG_64BIT
66+
67+
/*
68+
* Jan Achrenius on G+: microoptimized version of
69+
* the simpler "(mask & ONEBYTES) * ONEBYTES >> 56"
70+
* that works for the bytemasks without having to
71+
* mask them first.
72+
*/
73+
static inline long count_masked_bytes(unsigned long mask)
74+
{
75+
return mask*0x0001020304050608ul >> 56;
76+
}
77+
78+
#else /* 32-bit case */
79+
80+
/* Carl Chatfield / Jan Achrenius G+ version for 32-bit */
81+
static inline long count_masked_bytes(long mask)
82+
{
83+
/* (000000 0000ff 00ffff ffffff) -> ( 1 1 2 3 ) */
84+
long a = (0x0ff0001+mask) >> 23;
85+
/* Fix the 1 for 00 case */
86+
return a & mask;
87+
}
88+
89+
#endif
90+
91+
/* Return nonzero if it has a zero */
92+
static inline unsigned long has_zero(unsigned long a, unsigned long *bits, const struct word_at_a_time *c)
93+
{
94+
unsigned long mask = ((a - c->one_bits) & ~a) & c->high_bits;
95+
*bits = mask;
96+
return mask;
97+
}
98+
99+
static inline unsigned long prep_zero_mask(unsigned long a, unsigned long bits, const struct word_at_a_time *c)
100+
{
101+
return bits;
102+
}
103+
104+
static inline unsigned long create_zero_mask(unsigned long bits)
105+
{
106+
bits = (bits - 1) & ~bits;
107+
return bits >> 7;
108+
}
109+
110+
/* The mask we created is directly usable as a bytemask */
111+
#define zero_bytemask(mask) (mask)
112+
113+
static inline unsigned long find_zero(unsigned long mask)
114+
{
115+
return count_masked_bytes(mask);
116+
}
117+
118+
#endif /* __BIG_ENDIAN */
119+
56120
#endif /* _ASM_WORD_AT_A_TIME_H */

include/linux/string.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ extern char * strncpy(char *,const char *, __kernel_size_t);
2525
#ifndef __HAVE_ARCH_STRLCPY
2626
size_t strlcpy(char *, const char *, size_t);
2727
#endif
28+
#ifndef __HAVE_ARCH_STRSCPY
29+
ssize_t __must_check strscpy(char *, const char *, size_t);
30+
#endif
2831
#ifndef __HAVE_ARCH_STRCAT
2932
extern char * strcat(char *, const char *);
3033
#endif

lib/string.c

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
#include <linux/bug.h>
2828
#include <linux/errno.h>
2929

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

153+
#ifndef __HAVE_ARCH_STRSCPY
154+
/**
155+
* strscpy - Copy a C-string into a sized buffer
156+
* @dest: Where to copy the string to
157+
* @src: Where to copy the string from
158+
* @count: Size of destination buffer
159+
*
160+
* Copy the string, or as much of it as fits, into the dest buffer.
161+
* The routine returns the number of characters copied (not including
162+
* the trailing NUL) or -E2BIG if the destination buffer wasn't big enough.
163+
* The behavior is undefined if the string buffers overlap.
164+
* The destination buffer is always NUL terminated, unless it's zero-sized.
165+
*
166+
* Preferred to strlcpy() since the API doesn't require reading memory
167+
* from the src string beyond the specified "count" bytes, and since
168+
* the return value is easier to error-check than strlcpy()'s.
169+
* In addition, the implementation is robust to the string changing out
170+
* from underneath it, unlike the current strlcpy() implementation.
171+
*
172+
* Preferred to strncpy() since it always returns a valid string, and
173+
* doesn't unnecessarily force the tail of the destination buffer to be
174+
* zeroed. If the zeroing is desired, it's likely cleaner to use strscpy()
175+
* with an overflow test, then just memset() the tail of the dest buffer.
176+
*/
177+
ssize_t strscpy(char *dest, const char *src, size_t count)
178+
{
179+
const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
180+
size_t max = count;
181+
long res = 0;
182+
183+
if (count == 0)
184+
return -E2BIG;
185+
186+
#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
187+
/*
188+
* If src is unaligned, don't cross a page boundary,
189+
* since we don't know if the next page is mapped.
190+
*/
191+
if ((long)src & (sizeof(long) - 1)) {
192+
size_t limit = PAGE_SIZE - ((long)src & (PAGE_SIZE - 1));
193+
if (limit < max)
194+
max = limit;
195+
}
196+
#else
197+
/* If src or dest is unaligned, don't do word-at-a-time. */
198+
if (((long) dest | (long) src) & (sizeof(long) - 1))
199+
max = 0;
200+
#endif
201+
202+
while (max >= sizeof(unsigned long)) {
203+
unsigned long c, data;
204+
205+
c = *(unsigned long *)(src+res);
206+
*(unsigned long *)(dest+res) = c;
207+
if (has_zero(c, &data, &constants)) {
208+
data = prep_zero_mask(c, data, &constants);
209+
data = create_zero_mask(data);
210+
return res + find_zero(data);
211+
}
212+
res += sizeof(unsigned long);
213+
count -= sizeof(unsigned long);
214+
max -= sizeof(unsigned long);
215+
}
216+
217+
while (count) {
218+
char c;
219+
220+
c = src[res];
221+
dest[res] = c;
222+
if (!c)
223+
return res;
224+
res++;
225+
count--;
226+
}
227+
228+
/* Hit buffer length without finding a NUL; force NUL-termination. */
229+
if (res)
230+
dest[res-1] = '\0';
231+
232+
return -E2BIG;
233+
}
234+
EXPORT_SYMBOL(strscpy);
235+
#endif
236+
149237
#ifndef __HAVE_ARCH_STRCAT
150238
/**
151239
* strcat - Append one %NUL-terminated string to another

0 commit comments

Comments
 (0)