Skip to content

Commit ed42391

Browse files
keesgregkh
authored andcommitted
fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL
[ Upstream commit d07c0ac ] With CONFIG_FORTIFY=y and CONFIG_UBSAN_LOCAL_BOUNDS=y enabled, we observe a runtime panic while running Android's Compatibility Test Suite's (CTS) android.hardware.input.cts.tests. This is stemming from a strlen() call in hidinput_allocate(). __compiletime_strlen() is implemented in terms of __builtin_object_size(), then does an array access to check for NUL-termination. A quirk of __builtin_object_size() is that for strings whose values are runtime dependent, __builtin_object_size(str, 1 or 0) returns the maximum size of possible values when those sizes are determinable at compile time. Example: static const char *v = "FOO BAR"; static const char *y = "FOO BA"; unsigned long x (int z) { // Returns 8, which is: // max(__builtin_object_size(v, 1), __builtin_object_size(y, 1)) return __builtin_object_size(z ? v : y, 1); } So when FORTIFY_SOURCE is enabled, the current implementation of __compiletime_strlen() will try to access beyond the end of y at runtime using the size of v. Mixed with UBSAN_LOCAL_BOUNDS we get a fault. hidinput_allocate() has a local C string whose value is control flow dependent on a switch statement, so __builtin_object_size(str, 1) evaluates to the maximum string length, making all other cases fault on the last character check. hidinput_allocate() could be cleaned up to avoid runtime calls to strlen() since the local variable can only have literal values, so there's no benefit to trying to fortify the strlen call site there. Perform a __builtin_constant_p() check against index 0 earlier in the macro to filter out the control-flow-dependant case. Add a KUnit test for checking the expected behavioral characteristics of FORTIFY_SOURCE internals. Cc: Nathan Chancellor <[email protected]> Cc: Tom Rix <[email protected]> Cc: Andrew Morton <[email protected]> Cc: Vlastimil Babka <[email protected]> Cc: "Steven Rostedt (Google)" <[email protected]> Cc: David Gow <[email protected]> Cc: Yury Norov <[email protected]> Cc: Masami Hiramatsu <[email protected]> Cc: Sander Vanheule <[email protected]> Cc: [email protected] Cc: [email protected] Reviewed-by: Nick Desaulniers <[email protected]> Tested-by: Android Treehugger Robot Link: https://android-review.googlesource.com/c/kernel/common/+/2206839 Co-developed-by: Nick Desaulniers <[email protected]> Signed-off-by: Nick Desaulniers <[email protected]> Signed-off-by: Kees Cook <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent bf873a2 commit ed42391

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

include/linux/fortify-string.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
1919
unsigned char *__p = (unsigned char *)(p); \
2020
size_t __ret = (size_t)-1; \
2121
size_t __p_size = __builtin_object_size(p, 1); \
22-
if (__p_size != (size_t)-1) { \
22+
if (__p_size != (size_t)-1 && \
23+
__builtin_constant_p(*__p)) { \
2324
size_t __p_len = __p_size - 1; \
2425
if (__builtin_constant_p(__p[__p_len]) && \
2526
__p[__p_len] == '\0') \

0 commit comments

Comments
 (0)