diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index 03de71f654c7..22a5e62c92ea 100644 --- a/Documentation/process/deprecated.rst +++ b/Documentation/process/deprecated.rst @@ -131,27 +131,32 @@ value of strcpy() was used, since strscpy() does not return a pointer to the destination, but rather a count of non-NUL bytes copied (or negative errno when it truncates). -strncpy() on NUL-terminated strings ------------------------------------ -Use of strncpy() does not guarantee that the destination buffer will -be NUL terminated. This can lead to various linear read overflows and -other misbehavior due to the missing termination. It also NUL-pads -the destination buffer if the source contents are shorter than the -destination buffer size, which may be a needless performance penalty -for callers using only NUL-terminated strings. +strncpy() +--------- +strncpy() has been removed from the kernel. All former callers have +been migrated to safer alternatives. -When the destination is required to be NUL-terminated, the replacement is -strscpy(), though care must be given to any cases where the return value -of strncpy() was used, since strscpy() does not return a pointer to the -destination, but rather a count of non-NUL bytes copied (or negative -errno when it truncates). Any cases still needing NUL-padding should -instead use strscpy_pad(). +strncpy() did not guarantee NUL-termination of the destination buffer, +leading to linear read overflows and other misbehavior. It also +unconditionally NUL-padded the destination, which was a needless +performance penalty for callers using only NUL-terminated strings. Due +to its various behaviors, it was an ambiguous API for determining what +an author's true intent was for the copy. -If a caller is using non-NUL-terminated strings, strtomem() should be -used, and the destinations should be marked with the `__nonstring -`_ -attribute to avoid future compiler warnings. For cases still needing -NUL-padding, strtomem_pad() can be used. +The replacements for strncpy() are: + +- strscpy() when the destination must be NUL-terminated. +- strscpy_pad() when the destination must be NUL-terminated and + zero-padded (e.g., structs crossing privilege boundaries). +- memtostr() for NUL-terminated destinations from non-NUL-terminated + fixed-width sources (with the `__nonstring` attribute on the source). +- memtostr_pad() for the same, but with zero-padding. +- strtomem() for non-NUL-terminated fixed-width destinations, with + the `__nonstring` attribute on the destination. +- strtomem_pad() for non-NUL-terminated destinations that also need + zero-padding. +- memcpy_and_pad() for bounded copies from potentially unterminated + sources where the destination size is a runtime value. strlcpy() --------- diff --git a/arch/alpha/include/asm/string.h b/arch/alpha/include/asm/string.h index f043f91ff988..7c7545a2319a 100644 --- a/arch/alpha/include/asm/string.h +++ b/arch/alpha/include/asm/string.h @@ -47,8 +47,6 @@ extern inline void *__memset(void *s, int c, size_t n) #define __HAVE_ARCH_STRCPY extern char * strcpy(char *,const char *); -#define __HAVE_ARCH_STRNCPY -extern char * strncpy(char *, const char *, size_t); #define __HAVE_ARCH_STRCAT extern char * strcat(char *, const char *); #define __HAVE_ARCH_STRNCAT diff --git a/arch/alpha/lib/strncpy.S b/arch/alpha/lib/strncpy.S deleted file mode 100644 index cb90cf022df3..000000000000 --- a/arch/alpha/lib/strncpy.S +++ /dev/null @@ -1,83 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * arch/alpha/lib/strncpy.S - * Contributed by Richard Henderson (rth@tamu.edu) - * - * Copy no more than COUNT bytes of the null-terminated string from - * SRC to DST. If SRC does not cover all of COUNT, the balance is - * zeroed. - * - * Or, rather, if the kernel cared about that weird ANSI quirk. This - * version has cropped that bit o' nastiness as well as assuming that - * __stxncpy is in range of a branch. - */ -#include - .set noat - .set noreorder - - .text - - .align 4 - .globl strncpy - .ent strncpy -strncpy: - .frame $30, 0, $26 - .prologue 0 - - mov $16, $0 # set return value now - beq $18, $zerolen - unop - bsr $23, __stxncpy # do the work of the copy - - unop - bne $18, $multiword # do we have full words left? - subq $24, 1, $3 # nope - subq $27, 1, $4 - - or $3, $24, $3 # clear the bits between the last - or $4, $27, $4 # written byte and the last byte in COUNT - andnot $3, $4, $4 - zap $1, $4, $1 - - stq_u $1, 0($16) - ret - - .align 4 -$multiword: - subq $27, 1, $2 # clear the final bits in the prev word - or $2, $27, $2 - zapnot $1, $2, $1 - subq $18, 1, $18 - - stq_u $1, 0($16) - addq $16, 8, $16 - unop - beq $18, 1f - - nop - unop - nop - blbc $18, 0f - - stq_u $31, 0($16) # zero one word - subq $18, 1, $18 - addq $16, 8, $16 - beq $18, 1f - -0: stq_u $31, 0($16) # zero two words - subq $18, 2, $18 - stq_u $31, 8($16) - addq $16, 16, $16 - bne $18, 0b - -1: ldq_u $1, 0($16) # clear the leading bits in the final word - subq $24, 1, $2 - or $2, $24, $2 - - zap $1, $2, $1 - stq_u $1, 0($16) -$zerolen: - ret - - .end strncpy - EXPORT_SYMBOL(strncpy) diff --git a/arch/alpha/lib/styncpy.S b/arch/alpha/lib/styncpy.S index 72fc2754eb57..f1078ff12530 100644 --- a/arch/alpha/lib/styncpy.S +++ b/arch/alpha/lib/styncpy.S @@ -1,4 +1,3 @@ -#include "strncpy.S" #ifdef CONFIG_ALPHA_EV67 #include "ev67-strncat.S" #else diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h index 760cc13acdf4..ae6a6b51ad07 100644 --- a/arch/m68k/include/asm/string.h +++ b/arch/m68k/include/asm/string.h @@ -21,23 +21,6 @@ static inline size_t strnlen(const char *s, size_t count) return sc - s; } -#define __HAVE_ARCH_STRNCPY -static inline char *strncpy(char *dest, const char *src, size_t n) -{ - char *xdest = dest; - - asm volatile ("\n" - " jra 2f\n" - "1: move.b (%1),(%0)+\n" - " jeq 2f\n" - " addq.l #1,%1\n" - "2: subq.l #1,%2\n" - " jcc 1b\n" - : "+a" (dest), "+a" (src), "+d" (n) - : : "memory"); - return xdest; -} - #define __HAVE_ARCH_MEMMOVE extern void *memmove(void *, const void *, __kernel_size_t); diff --git a/arch/powerpc/boot/string.S b/arch/powerpc/boot/string.S index d2a2dbf1eefc..76801eb6d937 100644 --- a/arch/powerpc/boot/string.S +++ b/arch/powerpc/boot/string.S @@ -18,19 +18,6 @@ strcpy: bne 1b blr - .globl strncpy -strncpy: - cmpwi 0,r5,0 - beqlr - mtctr r5 - addi r6,r3,-1 - addi r4,r4,-1 -1: lbzu r0,1(r4) - cmpwi 0,r0,0 - stbu r0,1(r6) - bdnzf 2,1b /* dec ctr, branch if ctr != 0 && !cr0.eq */ - blr - .globl strcat strcat: addi r5,r3,-1 diff --git a/arch/powerpc/boot/string.h b/arch/powerpc/boot/string.h index 8c2ec0c05e4e..2b28d185343c 100644 --- a/arch/powerpc/boot/string.h +++ b/arch/powerpc/boot/string.h @@ -4,7 +4,6 @@ #include extern char *strcpy(char *dest, const char *src); -extern char *strncpy(char *dest, const char *src, size_t n); extern char *strcat(char *dest, const char *src); extern char *strchr(const char *s, int c); extern char *strrchr(const char *s, int c); diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h index 60ba22770f51..1981bd4036b5 100644 --- a/arch/powerpc/include/asm/string.h +++ b/arch/powerpc/include/asm/string.h @@ -5,7 +5,6 @@ #ifdef __KERNEL__ #ifndef CONFIG_KASAN -#define __HAVE_ARCH_STRNCPY #define __HAVE_ARCH_STRNCMP #define __HAVE_ARCH_MEMCHR #define __HAVE_ARCH_MEMCMP @@ -18,7 +17,6 @@ #define __HAVE_ARCH_MEMCPY_FLUSHCACHE extern char * strcpy(char *,const char *); -extern char * strncpy(char *,const char *, __kernel_size_t); extern __kernel_size_t strlen(const char *); extern int strcmp(const char *,const char *); extern int strncmp(const char *, const char *, __kernel_size_t); diff --git a/arch/powerpc/lib/string.S b/arch/powerpc/lib/string.S index daa72061dc0c..c3f5e7d31ee7 100644 --- a/arch/powerpc/lib/string.S +++ b/arch/powerpc/lib/string.S @@ -10,28 +10,6 @@ .text -/* This clears out any unused part of the destination buffer, - just as the libc version does. -- paulus */ -_GLOBAL(strncpy) - PPC_LCMPI 0,r5,0 - beqlr - mtctr r5 - addi r6,r3,-1 - addi r4,r4,-1 - .balign IFETCH_ALIGN_BYTES -1: lbzu r0,1(r4) - cmpwi 0,r0,0 - stbu r0,1(r6) - bdnzf 2,1b /* dec ctr, branch if ctr != 0 && !cr0.eq */ - bnelr /* if we didn't hit a null char, we're done */ - mfctr r5 - PPC_LCMPI 0,r5,0 /* any space left in destination buffer? */ - beqlr /* we know r0 == 0 here */ -2: stbu r0,1(r6) /* clear it out if so */ - bdnz 2b - blr -EXPORT_SYMBOL(strncpy) - _GLOBAL(strncmp) PPC_LCMPI 0,r5,0 beq- 2f diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h index e9cce169bb4c..43ab6806ab53 100644 --- a/arch/x86/include/asm/string_32.h +++ b/arch/x86/include/asm/string_32.h @@ -9,9 +9,6 @@ #define __HAVE_ARCH_STRCPY extern char *strcpy(char *dest, const char *src); -#define __HAVE_ARCH_STRNCPY -extern char *strncpy(char *dest, const char *src, size_t count); - #define __HAVE_ARCH_STRCAT extern char *strcat(char *dest, const char *src); diff --git a/arch/x86/lib/string_32.c b/arch/x86/lib/string_32.c index f87ec24fa579..a9480d73c4f0 100644 --- a/arch/x86/lib/string_32.c +++ b/arch/x86/lib/string_32.c @@ -30,25 +30,6 @@ char *strcpy(char *dest, const char *src) EXPORT_SYMBOL(strcpy); #endif -#ifdef __HAVE_ARCH_STRNCPY -char *strncpy(char *dest, const char *src, size_t count) -{ - int d0, d1, d2, d3; - asm volatile("1:\tdecl %2\n\t" - "js 2f\n\t" - "lodsb\n\t" - "stosb\n\t" - "testb %%al,%%al\n\t" - "jne 1b\n\t" - "rep stosb\n" - "2:" - : "=&S" (d0), "=&D" (d1), "=&c" (d2), "=&a" (d3) - : "0" (src), "1" (dest), "2" (count) : "memory"); - return dest; -} -EXPORT_SYMBOL(strncpy); -#endif - #ifdef __HAVE_ARCH_STRCAT char *strcat(char *dest, const char *src) { diff --git a/arch/xtensa/include/asm/string.h b/arch/xtensa/include/asm/string.h index ffce43513fa2..e16cda026df7 100644 --- a/arch/xtensa/include/asm/string.h +++ b/arch/xtensa/include/asm/string.h @@ -34,31 +34,6 @@ static inline char *strcpy(char *__dest, const char *__src) return __xdest; } -#define __HAVE_ARCH_STRNCPY -static inline char *strncpy(char *__dest, const char *__src, size_t __n) -{ - register char *__xdest = __dest; - unsigned long __dummy; - - if (__n == 0) - return __xdest; - - __asm__ __volatile__( - "1:\n\t" - "l8ui %2, %1, 0\n\t" - "s8i %2, %0, 0\n\t" - "addi %1, %1, 1\n\t" - "addi %0, %0, 1\n\t" - "beqz %2, 2f\n\t" - "bne %1, %5, 1b\n" - "2:" - : "=r" (__dest), "=r" (__src), "=&r" (__dummy) - : "0" (__dest), "1" (__src), "r" ((uintptr_t)__src+__n) - : "memory"); - - return __xdest; -} - #define __HAVE_ARCH_STRCMP static inline int strcmp(const char *__cs, const char *__ct) { diff --git a/drivers/misc/lkdtm/fortify.c b/drivers/misc/lkdtm/fortify.c index 7615a02dfc47..5e66707e1180 100644 --- a/drivers/misc/lkdtm/fortify.c +++ b/drivers/misc/lkdtm/fortify.c @@ -98,7 +98,7 @@ static void lkdtm_FORTIFY_MEM_MEMBER(void) pr_info("trying to memcpy() past the end of a struct member...\n"); /* - * strncpy(target.a, src, 20); will hit a compile error because the + * memcpy(target.a, src, 20); will hit a compile error because the * compiler knows at build time that target.a < 20 bytes. Use a * volatile to force a runtime error. */ diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 171982e53c9a..cf841dc71fef 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -26,7 +26,6 @@ #define FORTIFY_WRITE 1 #define EACH_FORTIFY_FUNC(macro) \ - macro(strncpy), \ macro(strnlen), \ macro(strlen), \ macro(strscpy), \ @@ -95,8 +94,6 @@ extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat); extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy); extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen); extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat); -extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy); - #else #if defined(__SANITIZE_MEMORY__) @@ -120,7 +117,6 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) #define __underlying_strcpy __builtin_strcpy #define __underlying_strlen __builtin_strlen #define __underlying_strncat __builtin_strncat -#define __underlying_strncpy __builtin_strncpy #endif @@ -159,50 +155,6 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) (bounds) < (length) \ ) -/** - * strncpy - Copy a string to memory with non-guaranteed NUL padding - * - * @p: pointer to destination of copy - * @q: pointer to NUL-terminated source string to copy - * @size: bytes to write at @p - * - * If strlen(@q) >= @size, the copy of @q will stop after @size bytes, - * and @p will NOT be NUL-terminated - * - * If strlen(@q) < @size, following the copy of @q, trailing NUL bytes - * will be written to @p until @size total bytes have been written. - * - * Do not use this function. While FORTIFY_SOURCE tries to avoid - * over-reads of @q, it cannot defend against writing unterminated - * results to @p. Using strncpy() remains ambiguous and fragile. - * Instead, please choose an alternative, so that the expectation - * of @p's contents is unambiguous: - * - * +--------------------+--------------------+------------+ - * | **p** needs to be: | padded to **size** | not padded | - * +====================+====================+============+ - * | NUL-terminated | strscpy_pad() | strscpy() | - * +--------------------+--------------------+------------+ - * | not NUL-terminated | strtomem_pad() | strtomem() | - * +--------------------+--------------------+------------+ - * - * Note strscpy*()'s differing return values for detecting truncation, - * and strtomem*()'s expectation that the destination is marked with - * __nonstring when it is a character array. - * - */ -__FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3) -char *strncpy(char * const POS p, const char *q, __kernel_size_t size) -{ - const size_t p_size = __member_size(p); - - if (__compiletime_lessthan(p_size, size)) - __write_overflow(); - if (p_size < size) - fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE, p_size, size, p); - return __underlying_strncpy(p, q, size); -} - extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen); /** * strnlen - Return bounded count of characters in a NUL-terminated string @@ -809,7 +761,6 @@ char *strcpy(char * const POS p, const char * const POS q) #undef __underlying_strcpy #undef __underlying_strlen #undef __underlying_strncat -#undef __underlying_strncpy #undef POS #undef POS0 diff --git a/include/linux/string.h b/include/linux/string.h index b850bd91b3d8..5702daca4326 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -67,9 +67,6 @@ void *vmemdup_array_user(const void __user *src, size_t n, size_t size) #ifndef __HAVE_ARCH_STRCPY extern char * strcpy(char *,const char *); #endif -#ifndef __HAVE_ARCH_STRNCPY -extern char * strncpy(char *,const char *, __kernel_size_t); -#endif ssize_t sized_strscpy(char *, const char *, size_t); /* diff --git a/lib/string.c b/lib/string.c index b632c71df1a5..45ca1cfe0184 100644 --- a/lib/string.c +++ b/lib/string.c @@ -88,22 +88,6 @@ char *strcpy(char *dest, const char *src) EXPORT_SYMBOL(strcpy); #endif -#ifndef __HAVE_ARCH_STRNCPY -char *strncpy(char *dest, const char *src, size_t count) -{ - char *tmp = dest; - - while (count) { - if ((*tmp = *src) != 0) - src++; - tmp++; - count--; - } - return dest; -} -EXPORT_SYMBOL(strncpy); -#endif - #ifdef __BIG_ENDIAN # define ALLBUTLAST_BYTE_MASK (~255ul) #else diff --git a/lib/test_fortify/write_overflow-strncpy-src.c b/lib/test_fortify/write_overflow-strncpy-src.c deleted file mode 100644 index 8dcfb8c788dd..000000000000 --- a/lib/test_fortify/write_overflow-strncpy-src.c +++ /dev/null @@ -1,5 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -#define TEST \ - strncpy(small, large_src, sizeof(small) + 1) - -#include "test_fortify.h" diff --git a/lib/test_fortify/write_overflow-strncpy.c b/lib/test_fortify/write_overflow-strncpy.c deleted file mode 100644 index b85f079c815d..000000000000 --- a/lib/test_fortify/write_overflow-strncpy.c +++ /dev/null @@ -1,5 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -#define TEST \ - strncpy(instance.buf, large_src, sizeof(instance.buf) + 1) - -#include "test_fortify.h" diff --git a/lib/tests/fortify_kunit.c b/lib/tests/fortify_kunit.c index fc9c76f026d6..413cdbf3dc0d 100644 --- a/lib/tests/fortify_kunit.c +++ b/lib/tests/fortify_kunit.c @@ -458,7 +458,7 @@ static void fortify_test_strnlen(struct kunit *test) /* Make string unterminated, and recount. */ pad.buf[end] = 'A'; end = sizeof(pad.buf); - /* Reading beyond with strncpy() will fail. */ + /* Reading beyond will fail. */ KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end); KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1); KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end); @@ -531,64 +531,6 @@ static void fortify_test_strcpy(struct kunit *test) KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); } -static void fortify_test_strncpy(struct kunit *test) -{ - struct fortify_padding pad = { }; - char src[] = "Copy me fully into a small buffer and I will overflow!"; - size_t sizeof_buf = sizeof(pad.buf); - - OPTIMIZER_HIDE_VAR(sizeof_buf); - - /* Destination is %NUL-filled to start with. */ - KUNIT_EXPECT_EQ(test, pad.bytes_before, 0); - KUNIT_EXPECT_EQ(test, pad.buf[sizeof_buf - 1], '\0'); - KUNIT_EXPECT_EQ(test, pad.buf[sizeof_buf - 2], '\0'); - KUNIT_EXPECT_EQ(test, pad.buf[sizeof_buf - 3], '\0'); - KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); - - /* Legitimate strncpy() 1 less than of max size. */ - KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src, sizeof_buf - 1) - == pad.buf); - KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); - /* Only last byte should be %NUL */ - KUNIT_EXPECT_EQ(test, pad.buf[sizeof_buf - 1], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 3], '\0'); - - /* Legitimate (though unterminated) max-size strncpy. */ - KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src, sizeof_buf) - == pad.buf); - KUNIT_EXPECT_EQ(test, fortify_write_overflows, 0); - /* No trailing %NUL -- thanks strncpy API. */ - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 1], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - /* But we will not have gone beyond. */ - KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); - - /* Now verify that FORTIFY is working... */ - KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src, sizeof_buf + 1) - == pad.buf); - /* Should catch the overflow. */ - KUNIT_EXPECT_EQ(test, fortify_write_overflows, 1); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 1], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - /* And we will not have gone beyond. */ - KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); - - /* And further... */ - KUNIT_ASSERT_TRUE(test, strncpy(pad.buf, src, sizeof_buf + 2) - == pad.buf); - /* Should catch the overflow. */ - KUNIT_EXPECT_EQ(test, fortify_write_overflows, 2); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 1], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - KUNIT_EXPECT_NE(test, pad.buf[sizeof_buf - 2], '\0'); - /* And we will not have gone beyond. */ - KUNIT_EXPECT_EQ(test, pad.bytes_after, 0); -} - static void fortify_test_strscpy(struct kunit *test) { struct fortify_padding pad = { }; @@ -1100,7 +1042,6 @@ static struct kunit_case fortify_test_cases[] = { KUNIT_CASE(fortify_test_strlen), KUNIT_CASE(fortify_test_strnlen), KUNIT_CASE(fortify_test_strcpy), - KUNIT_CASE(fortify_test_strncpy), KUNIT_CASE(fortify_test_strscpy), KUNIT_CASE(fortify_test_strcat), KUNIT_CASE(fortify_test_strncat),