[PATCH] fortify: Improve buffer overflow reporting

From: Kees Cook
Date: Thu Mar 02 2023 - 17:58:21 EST


Improve the reporting of buffer overflows under CONFIG_FORTIFY_SOURCE to
help accelerate debugging efforts. The calculations are all just sitting
in registers anyway, so pass them along to the function to be reported.

For example, before:

detected buffer overflow in memcpy

and after:

memcpy: detected buffer overflow: 4096 byte read from buffer of size 1

Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
Cc: Andy Shevchenko <andy@xxxxxxxxxx>
Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Cc: Nicolas Schier <nicolas@xxxxxxxxx>
Cc: Tom Rix <trix@xxxxxxxxxx>
Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Cc: Miroslav Benes <mbenes@xxxxxxx>
Cc: Marco Elver <elver@xxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Cc: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
Cc: Mark Brown <broonie@xxxxxxxxxx>
Cc: Puyou Lu <puyou.lu@xxxxxxxxx>
Cc: linux-hardening@xxxxxxxxxxxxxxx
Cc: linux-kbuild@xxxxxxxxxxxxxxx
Cc: llvm@xxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
include/linux/fortify-string.h | 51 +++++++++++++++++++---------------
lib/string_helpers.c | 5 ++--
2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c9de1f59ee80..981e2838f99a 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -9,7 +9,7 @@
#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
#define __RENAME(x) __asm__(#x)

-void fortify_panic(const char *name) __noreturn __cold;
+void fortify_panic(const char *name, bool dest, size_t avail, size_t len) __noreturn __cold;
void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
@@ -147,7 +147,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__write_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
return __underlying_strncpy(p, q, size);
}

@@ -170,11 +170,13 @@ __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
char *strcat(char * const POS p, const char *q)
{
size_t p_size = __member_size(p);
+ size_t size;

if (p_size == SIZE_MAX)
return __underlying_strcat(p, q);
- if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(__func__);
+ size = strlcat(p, q, p_size);
+ if (p_size < size)
+ fortify_panic(__func__, 1, p_size, size);
return p;
}

@@ -205,7 +207,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
/* Do not check characters beyond the end of p. */
ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
if (p_size <= ret && maxlen != ret)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, ret);
return ret;
}

@@ -241,7 +243,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, ret);
return ret;
}

@@ -282,8 +284,8 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
__write_overflow();
}
if (size) {
- if (len >= p_size)
- fortify_panic(__func__);
+ if (p_size < len)
+ fortify_panic(__func__, 1, p_size, len);
__underlying_memcpy(p, q, len);
p[len] = '\0';
}
@@ -361,7 +363,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
* p_size.
*/
if (len > p_size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, len);

/*
* We can now safely call vanilla strscpy because we are protected from:
@@ -397,13 +399,15 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
size_t p_len, copy_len;
size_t p_size = __member_size(p);
size_t q_size = __member_size(q);
+ size_t size;

if (p_size == SIZE_MAX && q_size == SIZE_MAX)
return __underlying_strncat(p, q, count);
p_len = strlen(p);
copy_len = strnlen(q, count);
- if (p_size < p_len + copy_len + 1)
- fortify_panic(__func__);
+ size = p_len + copy_len + 1;
+ if (p_size < size)
+ fortify_panic(__func__, 1, p_size, size);
__underlying_memcpy(p + p_len, q, copy_len);
p[p_len + copy_len] = '\0';
return p;
@@ -444,7 +448,7 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic("memset");
+ fortify_panic("memset", 1, p_size, size);
}

#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
@@ -542,9 +546,10 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
* (The SIZE_MAX test is to optimize away checks where the buffer
* lengths are unknown.)
*/
- if ((p_size != SIZE_MAX && p_size < size) ||
- (q_size != SIZE_MAX && q_size < size))
- fortify_panic(func);
+ if (p_size != SIZE_MAX && p_size < size)
+ fortify_panic(func, 1, p_size, size);
+ if (q_size != SIZE_MAX && q_size < size)
+ fortify_panic(func, 0, q_size, size);

/*
* Warn when writing beyond destination field size.
@@ -644,7 +649,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
return __real_memscan(p, c, size);
}

@@ -660,8 +665,10 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
if (__compiletime_lessthan(q_size, size))
__read_overflow2();
}
- if (p_size < size || q_size < size)
- fortify_panic(__func__);
+ if (p_size < size)
+ fortify_panic(__func__, 1, p_size, size);
+ if (q_size < size)
+ fortify_panic(__func__, 0, q_size, size);
return __underlying_memcmp(p, q, size);
}

@@ -673,7 +680,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
return __underlying_memchr(p, c, size);
}

@@ -685,7 +692,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
return __real_memchr_inv(p, c, size);
}

@@ -698,7 +705,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
return __real_kmemdup(p, size, gfp);
}

@@ -735,7 +742,7 @@ char *strcpy(char * const POS p, const char * const POS q)
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(__func__, 1, p_size, size);
__underlying_memcpy(p, q, size);
return p;
}
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 230020a2e076..b2d3e1d3931e 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -1021,9 +1021,10 @@ EXPORT_SYMBOL(__read_overflow2_field);
void __write_overflow_field(size_t avail, size_t wanted) { }
EXPORT_SYMBOL(__write_overflow_field);

-void fortify_panic(const char *name)
+void fortify_panic(const char *name, bool dest, size_t avail, size_t len)
{
- pr_emerg("detected buffer overflow in %s\n", name);
+ pr_emerg("%s: detected buffer overflow: %zu byte %s buffer of size %zu\n",
+ name, len, dest ? "write to" : "read from", avail);
BUG();
}
EXPORT_SYMBOL(fortify_panic);
--
2.34.1