From ead62aa370a81c4fb42a44c4edeafe13e0a3a703 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sat, 13 May 2023 11:18:40 +0200 Subject: fortify: strscpy: Fix flipped q and p docstring typo Fix typo in the strscpy() docstring where q and p were flipped. Signed-off-by: Arne Welzel Signed-off-by: Kees Cook --- include/linux/fortify-string.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/linux/fortify-string.h') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index c9de1f59ee80..e29df83bff8a 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -299,8 +299,8 @@ extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy); * @q: Where to copy the string from * @size: Size of destination buffer * - * Copy the source string @p, or as much of it as fits, into the destination - * @q buffer. The behavior is undefined if the string buffers overlap. The + * Copy the source string @q, or as much of it as fits, into the destination + * @p buffer. The behavior is undefined if the string buffers overlap. The * destination @p buffer is always NUL terminated, unless it's zero-sized. * * Preferred to strlcpy() since the API doesn't require reading memory -- cgit v1.2.3 From 21a2c74b0a2a784228c9e3af63cff96d0dea7b8a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 7 Apr 2023 12:27:10 -0700 Subject: fortify: Use const variables for __member_size tracking The sizes reported by __member_size should never change in a given function. Mark them as such. Suggested-by: Miguel Ojeda Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook Reviewed-by: Nick Desaulniers Link: https://lore.kernel.org/r/20230407192717.636137-4-keescook@chromium.org --- include/linux/fortify-string.h | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'include/linux/fortify-string.h') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index e29df83bff8a..2e7a47b62ba2 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -20,7 +20,7 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning(" ({ \ char *__p = (char *)(p); \ size_t __ret = SIZE_MAX; \ - size_t __p_size = __member_size(p); \ + const size_t __p_size = __member_size(p); \ if (__p_size != SIZE_MAX && \ __builtin_constant_p(*__p)) { \ size_t __p_len = __p_size - 1; \ @@ -142,7 +142,7 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __FORTIFY_INLINE __diagnose_as(__builtin_strncpy, 1, 2, 3) char *strncpy(char * const POS p, const char *q, __kernel_size_t size) { - size_t p_size = __member_size(p); + const size_t p_size = __member_size(p); if (__compiletime_lessthan(p_size, size)) __write_overflow(); @@ -169,7 +169,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size) __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2) char *strcat(char * const POS p, const char *q) { - size_t p_size = __member_size(p); + const size_t p_size = __member_size(p); if (p_size == SIZE_MAX) return __underlying_strcat(p, q); @@ -191,8 +191,8 @@ extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(st */ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen) { - size_t p_size = __member_size(p); - size_t p_len = __compiletime_strlen(p); + const size_t p_size = __member_size(p); + const size_t p_len = __compiletime_strlen(p); size_t ret; /* We can take compile-time actions when maxlen is const. */ @@ -233,8 +233,8 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size __FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1) __kernel_size_t __fortify_strlen(const char * const POS p) { + const size_t p_size = __member_size(p); __kernel_size_t ret; - size_t p_size = __member_size(p); /* Give up if we don't know how large p is. */ if (p_size == SIZE_MAX) @@ -267,8 +267,8 @@ extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy); */ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, size_t size) { - size_t p_size = __member_size(p); - size_t q_size = __member_size(q); + const size_t p_size = __member_size(p); + const size_t q_size = __member_size(q); size_t q_len; /* Full count of source string length. */ size_t len; /* Count of characters going into destination. */ @@ -318,10 +318,10 @@ extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy); */ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, size_t size) { - size_t len; /* Use string size rather than possible enclosing struct size. */ - size_t p_size = __member_size(p); - size_t q_size = __member_size(q); + const size_t p_size = __member_size(p); + const size_t q_size = __member_size(q); + size_t len; /* If we cannot get size of p and q default to call strscpy. */ if (p_size == SIZE_MAX && q_size == SIZE_MAX) @@ -394,9 +394,9 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s __FORTIFY_INLINE __diagnose_as(__builtin_strncat, 1, 2, 3) char *strncat(char * const POS p, const char * const POS q, __kernel_size_t count) { + const size_t p_size = __member_size(p); + const size_t q_size = __member_size(q); size_t p_len, copy_len; - size_t p_size = __member_size(p); - size_t q_size = __member_size(q); if (p_size == SIZE_MAX && q_size == SIZE_MAX) return __underlying_strncat(p, q, count); @@ -639,7 +639,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size, extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan); __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size) { - size_t p_size = __struct_size(p); + const size_t p_size = __struct_size(p); if (__compiletime_lessthan(p_size, size)) __read_overflow(); @@ -651,8 +651,8 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size) __FORTIFY_INLINE __diagnose_as(__builtin_memcmp, 1, 2, 3) int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t size) { - size_t p_size = __struct_size(p); - size_t q_size = __struct_size(q); + const size_t p_size = __struct_size(p); + const size_t q_size = __struct_size(q); if (__builtin_constant_p(size)) { if (__compiletime_lessthan(p_size, size)) @@ -668,7 +668,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t __FORTIFY_INLINE __diagnose_as(__builtin_memchr, 1, 2, 3) void *memchr(const void * const POS0 p, int c, __kernel_size_t size) { - size_t p_size = __struct_size(p); + const size_t p_size = __struct_size(p); if (__compiletime_lessthan(p_size, size)) __read_overflow(); @@ -680,7 +680,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size) void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv); __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size) { - size_t p_size = __struct_size(p); + const size_t p_size = __struct_size(p); if (__compiletime_lessthan(p_size, size)) __read_overflow(); @@ -693,7 +693,7 @@ extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kme __realloc_size(2); __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp) { - size_t p_size = __struct_size(p); + const size_t p_size = __struct_size(p); if (__compiletime_lessthan(p_size, size)) __read_overflow(); @@ -720,8 +720,8 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp __FORTIFY_INLINE __diagnose_as(__builtin_strcpy, 1, 2) char *strcpy(char * const POS p, const char * const POS q) { - size_t p_size = __member_size(p); - size_t q_size = __member_size(q); + const size_t p_size = __member_size(p); + const size_t q_size = __member_size(q); size_t size; /* If neither buffer size is known, immediately give up. */ -- cgit v1.2.3 From 605395cd7ceded5842c8ba6763ea24feee690c87 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 2 Apr 2023 23:00:05 -0700 Subject: fortify: Add protection for strlcat() The definition of strcat() was defined in terms of unfortified strlcat(), but that meant there was no bounds checking done on the internal strlen() calls, and the (bounded) copy would be performed before reporting a failure. Additionally, pathological cases (i.e. unterminated destination buffer) did not make calls to fortify_panic(), which will make future unit testing more difficult. Instead, explicitly define a fortified strlcat() wrapper for strcat() to use. Signed-off-by: Kees Cook --- include/linux/fortify-string.h | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) (limited to 'include/linux/fortify-string.h') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 2e7a47b62ba2..756c89bb88e0 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -371,6 +371,70 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s return __real_strscpy(p, q, len); } +/* Defined after fortified strlen() to reuse it. */ +extern size_t __real_strlcat(char *p, const char *q, size_t avail) __RENAME(strlcat); +/** + * strlcat - Append a string to an existing string + * + * @p: pointer to %NUL-terminated string to append to + * @q: pointer to %NUL-terminated string to append from + * @avail: Maximum bytes available in @p + * + * Appends %NUL-terminated string @q after the %NUL-terminated + * string at @p, but will not write beyond @avail bytes total, + * potentially truncating the copy from @q. @p will stay + * %NUL-terminated only if a %NUL already existed within + * the @avail bytes of @p. If so, the resulting number of + * bytes copied from @q will be at most "@avail - strlen(@p) - 1". + * + * Do not use this function. While FORTIFY_SOURCE tries to avoid + * read and write overflows, this is only possible when the sizes + * of @p and @q are known to the compiler. Prefer building the + * string with formatting, via scnprintf(), seq_buf, or similar. + * + * Returns total bytes that _would_ have been contained by @p + * regardless of truncation, similar to snprintf(). If return + * value is >= @avail, the string has been truncated. + * + */ +__FORTIFY_INLINE +size_t strlcat(char * const POS p, const char * const POS q, size_t avail) +{ + const size_t p_size = __member_size(p); + const size_t q_size = __member_size(q); + size_t p_len, copy_len; + size_t actual, wanted; + + /* Give up immediately if both buffer sizes are unknown. */ + if (p_size == SIZE_MAX && q_size == SIZE_MAX) + return __real_strlcat(p, q, avail); + + p_len = strnlen(p, avail); + copy_len = strlen(q); + wanted = actual = p_len + copy_len; + + /* Cannot append any more: report truncation. */ + if (avail <= p_len) + return wanted; + + /* Give up if string is already overflowed. */ + if (p_size <= p_len) + fortify_panic(__func__); + + if (actual >= avail) { + copy_len = avail - p_len - 1; + actual = p_len + copy_len; + } + + /* Give up if copy will overflow. */ + if (p_size <= actual) + fortify_panic(__func__); + __underlying_memcpy(p + p_len, q, copy_len); + p[actual] = '\0'; + + return wanted; +} + /** * strncat - Append a string to an existing string * -- cgit v1.2.3 From 55c84a5cf2c72a821719823ef2ebef01b119025b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 4 Apr 2023 14:24:27 -0700 Subject: fortify: strcat: Move definition to use fortified strlcat() Move the definition of fortified strcat() to after strlcat() to use it for bounds checking. Signed-off-by: Kees Cook --- include/linux/fortify-string.h | 53 +++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 27 deletions(-) (limited to 'include/linux/fortify-string.h') diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h index 756c89bb88e0..da51a83b2829 100644 --- a/include/linux/fortify-string.h +++ b/include/linux/fortify-string.h @@ -151,33 +151,6 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size) return __underlying_strncpy(p, q, size); } -/** - * strcat - Append a string to an existing string - * - * @p: pointer to NUL-terminated string to append to - * @q: pointer to NUL-terminated source string to append from - * - * Do not use this function. While FORTIFY_SOURCE tries to avoid - * read and write overflows, this is only possible when the - * destination buffer size is known to the compiler. Prefer - * building the string with formatting, via scnprintf() or similar. - * At the very least, use strncat(). - * - * Returns @p. - * - */ -__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2) -char *strcat(char * const POS p, const char *q) -{ - const size_t p_size = __member_size(p); - - if (p_size == SIZE_MAX) - return __underlying_strcat(p, q); - if (strlcat(p, q, p_size) >= p_size) - fortify_panic(__func__); - return p; -} - extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen); /** * strnlen - Return bounded count of characters in a NUL-terminated string @@ -435,6 +408,32 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail) return wanted; } +/* Defined after fortified strlcat() to reuse it. */ +/** + * strcat - Append a string to an existing string + * + * @p: pointer to NUL-terminated string to append to + * @q: pointer to NUL-terminated source string to append from + * + * Do not use this function. While FORTIFY_SOURCE tries to avoid + * read and write overflows, this is only possible when the + * destination buffer size is known to the compiler. Prefer + * building the string with formatting, via scnprintf() or similar. + * At the very least, use strncat(). + * + * Returns @p. + * + */ +__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2) +char *strcat(char * const POS p, const char *q) +{ + const size_t p_size = __member_size(p); + + if (strlcat(p, q, p_size) >= p_size) + fortify_panic(__func__); + return p; +} + /** * strncat - Append a string to an existing string * -- cgit v1.2.3