Fix some issues uncovered by UBSAN and enable UBSAN for nolibc-test to avoid regressions.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net --- Changes in v2: - Introduce and use __nolibc_aligned_as() - Reduce size of fixes to i{64,}toa_r() - Link to v1: https://lore.kernel.org/r/20250416-nolibc-ubsan-v1-0-c4704bb23da7@weissschuh...
--- Thomas Weißschuh (7): tools/nolibc: add __nolibc_has_feature() tools/nolibc: add __nolibc_aligned() and __nolibc_aligned_as() tools/nolibc: disable function sanitizer for _start_c() tools/nolibc: properly align dirent buffer tools/nolibc: fix integer overflow in i{64,}toa_r() and selftests/nolibc: disable ubsan for smash_stack() selftests/nolibc: enable UBSAN if available
tools/include/nolibc/compiler.h | 9 +++++++++ tools/include/nolibc/crt.h | 5 +++++ tools/include/nolibc/dirent.h | 3 ++- tools/include/nolibc/stdlib.h | 4 ++-- tools/testing/selftests/nolibc/Makefile | 3 ++- tools/testing/selftests/nolibc/nolibc-test.c | 1 + 6 files changed, 21 insertions(+), 4 deletions(-) --- base-commit: 7c73c10b906778384843b9d3ac6c2224727bbf5c change-id: 20250416-nolibc-ubsan-028401698654
Best regards,
Certain compiler features are signaled via the __has_feature() preprocessor builtin.
Add a nolibc wrapper for it, similar to __nolibc_has_attribute().
Signed-off-by: Thomas Weißschuh linux@weissschuh.net --- tools/include/nolibc/compiler.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h index fa1f547e7f13d151ea98b9c585b36659d2d27324..e6d6dc116e43aa69b37eca02ed1590fc09486bdb 100644 --- a/tools/include/nolibc/compiler.h +++ b/tools/include/nolibc/compiler.h @@ -12,6 +12,12 @@ # define __nolibc_has_attribute(attr) 0 #endif
+#if defined(__has_feature) +# define __nolibc_has_feature(feature) __has_feature(feature) +#else +# define __nolibc_has_feature(feature) 0 +#endif + #if __nolibc_has_attribute(naked) # define __nolibc_entrypoint __attribute__((naked)) # define __nolibc_entrypoint_epilogue()
Provide a convenience macro around __attribute__((aligned)).
Signed-off-by: Thomas Weißschuh linux@weissschuh.net
--- Willy, I used the name __nolibc_align*ed* because it matches what the underlying attribute and the kernel macros are using. --- tools/include/nolibc/compiler.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h index e6d6dc116e43aa69b37eca02ed1590fc09486bdb..369cfb5a0e78f4ae1e1a2cac4077024b4e8ba225 100644 --- a/tools/include/nolibc/compiler.h +++ b/tools/include/nolibc/compiler.h @@ -18,6 +18,9 @@ # define __nolibc_has_feature(feature) 0 #endif
+#define __nolibc_aligned(alignment) __attribute__((aligned(alignment))) +#define __nolibc_aligned_as(type) __nolibc_aligned(__alignof__(type)) + #if __nolibc_has_attribute(naked) # define __nolibc_entrypoint __attribute__((naked)) # define __nolibc_entrypoint_epilogue()
Both constructors and main() may be executed with different function signatures than they are actually using. This is intentional but trips up UBSAN.
Disable the function sanitizer of UBSAN in _start_c().
Signed-off-by: Thomas Weißschuh linux@weissschuh.net --- tools/include/nolibc/crt.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h index c4b10103bbec50f1a3a0a4562e34fdbd1b43ce6f..961cfe777c3564e705dfdd581de828b374d05b0b 100644 --- a/tools/include/nolibc/crt.h +++ b/tools/include/nolibc/crt.h @@ -7,6 +7,8 @@ #ifndef _NOLIBC_CRT_H #define _NOLIBC_CRT_H
+#include "compiler.h" + char **environ __attribute__((weak)); const unsigned long *_auxv __attribute__((weak));
@@ -25,6 +27,9 @@ extern void (*const __fini_array_end[])(void) __attribute__((weak));
void _start_c(long *sp); __attribute__((weak,used)) +#if __nolibc_has_feature(undefined_behavior_sanitizer) + __attribute__((no_sanitize("function"))) +#endif void _start_c(long *sp) { long argc;
As byte buffer is overlaid with a 'struct dirent64'. it has to satisfy the structs alignment requirements.
Fixes: 665fa8dea90d ("tools/nolibc: add support for directory access") Signed-off-by: Thomas Weißschuh linux@weissschuh.net Acked-by: Willy Tarreau w@1wt.eu --- tools/include/nolibc/dirent.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/include/nolibc/dirent.h b/tools/include/nolibc/dirent.h index c5c30d0dd6806b1bec2fa8120a3df29aaa201393..946a697e98e4c7bb0970b163ffdfe0e069b1d160 100644 --- a/tools/include/nolibc/dirent.h +++ b/tools/include/nolibc/dirent.h @@ -7,6 +7,7 @@ #ifndef _NOLIBC_DIRENT_H #define _NOLIBC_DIRENT_H
+#include "compiler.h" #include "stdint.h" #include "types.h"
@@ -58,7 +59,7 @@ int closedir(DIR *dirp) static __attribute__((unused)) int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result) { - char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1]; + char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1] __nolibc_aligned_as(struct linux_dirent64); struct linux_dirent64 *ldir = (void *)buf; intptr_t i = (intptr_t)dirp; int fd, ret;
In twos complement the most negative number can not be negated.
Fixes: b1c21e7d99cd ("tools/nolibc/stdlib: add i64toa() and u64toa()") Fixes: 66c397c4d2e1 ("tools/nolibc/stdlib: replace the ltoa() function with more efficient ones") Signed-off-by: Thomas Weißschuh linux@weissschuh.net --- tools/include/nolibc/stdlib.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h index 86ad378ab1ea220559d5ab1adc4bb9972977ba9e..32b3038002c16864cf66a71ae9fa3825f995b09c 100644 --- a/tools/include/nolibc/stdlib.h +++ b/tools/include/nolibc/stdlib.h @@ -275,7 +275,7 @@ int itoa_r(long in, char *buffer) int len = 0;
if (in < 0) { - in = -in; + in = -(unsigned long)in; *(ptr++) = '-'; len++; } @@ -411,7 +411,7 @@ int i64toa_r(int64_t in, char *buffer) int len = 0;
if (in < 0) { - in = -in; + in = -(uint64_t)in; *(ptr++) = '-'; len++; }
smash_stack() intentionally crashes.
Prevent UBSAN from tripping over it.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net --- tools/testing/selftests/nolibc/nolibc-test.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 7a60b6ac1457e8d862ab1a6a26c9e46abec92111..b176a706609b7641dd1d743c8a02b6b6e7a4c746 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -1438,6 +1438,7 @@ static int run_vfprintf(int min, int max) return ret; }
+__attribute__((no_sanitize("undefined"))) static int smash_stack(void) { char buf[100];
UBSAN detects undefined behaviour at runtime. To avoid introduction of new UB, enable UBSAN for nolibc-test.
By signalling detected errors through traps no runtime dependency is necessary.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net Acked-by: Willy Tarreau w@1wt.eu --- tools/testing/selftests/nolibc/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index aa05c1faac20d3054b231090b939d050b0231d40..94f3e8be7a68f63ecd639c4f283b3cd10764ce74 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -189,9 +189,10 @@ ifeq ($(origin XARCH),command line) CFLAGS_XARCH = $(CFLAGS_$(XARCH)) endif CFLAGS_STACKPROTECTOR ?= $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) +CFLAGS_SANITIZER ?= $(call cc-option,-fsanitize=undefined -fsanitize-trap=all) CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wextra \ $(call cc-option,-fno-stack-protector) $(call cc-option,-Wmissing-prototypes) \ - $(CFLAGS_XARCH) $(CFLAGS_STACKPROTECTOR) $(CFLAGS_EXTRA) + $(CFLAGS_XARCH) $(CFLAGS_STACKPROTECTOR) $(CFLAGS_SANITIZER) $(CFLAGS_EXTRA) LDFLAGS :=
LIBGCC := -lgcc
On Sat, Apr 19, 2025 at 12:46:17PM +0200, Thomas Weißschuh wrote:
Fix some issues uncovered by UBSAN and enable UBSAN for nolibc-test to avoid regressions.
Signed-off-by: Thomas Weißschuh linux@weissschuh.net
Changes in v2:
- Introduce and use __nolibc_aligned_as()
- Reduce size of fixes to i{64,}toa_r()
- Link to v1: https://lore.kernel.org/r/20250416-nolibc-ubsan-v1-0-c4704bb23da7@weissschuh...
Wow that was a fast turn-around! Looks good. For the whole series:
Acked-by: Willy Tarreau w@1wt.eu
Thank you Thomas! Willy
linux-kselftest-mirror@lists.linaro.org
-
Thomas Weißschuh -
Willy Tarreau