Detect signed integer wrap-around (overflow and underflow) #26

kees · 2019-12-05T19:34:09Z

Much like the array bounds checking (issue #25), the kernel should be detecting signed integer overflows using the Undefined Behavior Sanitizer (UBSan) compiler feature. There are some false positives that need to be fixed (e.g. commit a318f12), and there are some true positives that are expected (e.g. refcount_t overflow), and need to be marked.

There is, however, a complication with the kernel's use of -fno-strict-overflow which implies -fwrapv-pointer and -fwrapv (which is needed to keep the compiler from optimizing things away that are considered "undefined", when what is wanted is "expected" 2s-complement wrap-around on overflow). The former is for wrapped unsigned integer overflow (i.e. unsigned long pointer values), and the latter is needed for true positive signed overflow (i.e. refcount_t). However, this makes integer overflow no longer undefined behavior, making UBSan not catch overflows any more. :( To fix this, we need the "intentional overflow/wrap" helpers to DTRT in the face of -fno-wrapv so that UBSan will work correctly.

Language clarification:

"overflow" has a specific meaning related to undefined behavior
"wrap (around)" has a specific meaning related to the handling of signed overflow through wrap-around (i.e. defined behavior).

To avoid Undefined Behavior, the kernel must keep -fno-strict-overflow.

So, things to do:

make signed integer sanitizer work even with -fwrapv (and -fno-strict-overflow). done
create "expected signed overflow" helper inline functions marked with __attribute__((no_sanitize("signed-integer-overflow"))). done
add back signed integer overflow as a UBSan Kconfig done
add note to "deprecated.rst" with something like "open coded signed integer wrap around without a helper".
investigate need for -fsanitize=signed-integer-truncation and create new issue if needed
annote enough false positives that production workloads can run (e.g. Android, Ubuntu, etc).

The text was updated successfully, but these errors were encountered:

kees · 2021-08-12T20:14:16Z

To illustrate the issue:

#include <stdio.h>
#include <limits.h>

/* volatile to avoid optimization */
volatile int val;

int main(void)
{   
    val = INT_MAX; 
    val += 1;
    printf("%d\n", val);
    return 0;
}

Normally, the expected warning is generated:

$ gcc -Wall -O2 -fsanitize=signed-integer-overflow -o wrap wrap.c
$ ./wrap
wrap.c:10:9: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
-2147483648

But with -fno-strict-overflow the condition goes unnoticed:

$ gcc -Wall -O2 -fsanitize=signed-integer-overflow -fno-strict-overflow -o wrap wrap.c
$ ./wrap
-2147483648

kees · 2021-09-02T21:39:15Z

See also commit 6aaa31a

kees · 2021-09-14T15:58:00Z

Interaction with -fno-wrapv: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317

kees · 2021-09-14T16:36:29Z

A nice test for the UB (thank you to Miguel Ojeda) here is:

bool f(const int x) {
    return x + 1 > x;
}

UB will show this as always returning true.

kees · 2021-09-25T13:25:11Z

More notes/background: https://danluu.com/integer-overflow/

kees · 2022-01-28T23:09:43Z

Would have caught:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=722d94847de2

bwendling · 2023-09-20T23:51:38Z

Copying this GCC documentation here for my benefit:

-fstrict-overflow

Allow the compiler to assume strict signed overflow rules, depending on the language being compiled. For C (and C++) this means that overflow when doing arithmetic with signed numbers is undefined, which means that the compiler may assume that it does not happen. This permits various optimizations. For example, the compiler assumes that an expression like i + 10 > i is always true for signed i. This assumption is only valid if signed overflow is undefined, as the expression is false if i + 10 overflows when using twos complement arithmetic. When this option is in effect any attempt to determine whether an operation on signed numbers overflows must be written carefully to not actually involve overflow.

This option also allows the compiler to assume strict pointer semantics: given a pointer to an object, if adding an offset to that pointer does not produce a pointer to the same object, the addition is undefined. This permits the compiler to conclude that p + u > p is always true for a pointer p and unsigned integer u. This assumption is only valid because pointer wraparound is undefined, as the expression is false if p + u overflows using twos complement arithmetic.

See also the -fwrapv option. Using -fwrapv means that integer signed overflow is fully defined: it wraps. When -fwrapv is used, there is no difference between -fstrict-overflow and -fno-strict-overflow for integers. With -fwrapv certain types of overflow are permitted. For example, if the compiler gets an overflow when doing arithmetic on constants, the overflowed value can still be used with -fwrapv, but not otherwise.

The -fstrict-overflow option is enabled at levels -O2, -O3, -Os.

-fwrapv

This option instructs the compiler to assume that signed arithmetic overflow of addition, subtraction and multiplication wraps around using twos-complement representation. This flag enables some optimizations and disables others. This option is enabled by default for the Java front-end, as required by the Java language specification.

kees · 2023-09-25T01:38:51Z

Also:

-fwrapv-pointer

This option instructs the compiler to assume that pointer arithmetic overflow on addition and subtraction
wraps around using twos-complement representation. This flag disables some optimizations which assume pointer
overflow is invalid.

JustinStitt · 2024-01-08T21:31:54Z

Does anyone know why this sanitizer only works for some integer types and not all? Is this intentional?

See: https://godbolt.org/z/ET1Kz6YYn

Here it works fine with int but not with int8_t.

edit (01/30/2024): This is likely due to promotion rules (well-defined by the spec)

kees · 2024-01-08T22:59:34Z

Yet another bug! Yes, it seems that it is limited only to 32-bit signed overflow. :(

kees · 2024-01-09T19:42:05Z

Here's a recent thread with GCC folks where we clarified language and I tried to explain the kernel's behavioral needs:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630578.html

In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Switch to a more regular type for a 64-bit value and refactor the open-coded wrap-around addition test to use subtraction from the type max (since add_would_overflow() may not be defined in early boot code). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: KSPP#26 [2] Link: KSPP#27 [3] Link: KSPP#344 [4] Cc: Nick Terrell <terrelln@fb.com> Cc: Paul Jones <paul@pauljones.id.au> Cc: Sedat Dilek <sedat.dilek@gmail.com> Cc: Oleksandr Natalenko <oleksandr@natalenko.name> Cc: Xin Gao <gaoxin@cdjrlc.com> Signed-off-by: Kees Cook <keescook@chromium.org>

JustinStitt · 2024-01-31T00:59:30Z

Yet another bug! Yes, it seems that it is limited only to 32-bit signed overflow. :(

This is likely due to promotion rules; we talked about this offline just wanted to keep this thread up-to-date, somewhat 😸

…_user() The loop counter "i" in copy_compat_iovec_from_user() is an int, but because the nr_segs argument is unsigned long, the signed overflow sanitizer got worried "i" could wrap around. Instead of making "i" an unsigned long (which may enlarge the type size), switch both nr_segs and i to u32. There is no truncation with nr_segs since it is never larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled Link: KSPP#26 [1] Link: https://lkml.kernel.org/r/20240129183729.work.991-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

…_user() The loop counter "i" in copy_compat_iovec_from_user() is an int, but because the nr_segs argument is unsigned long, the signed overflow sanitizer got worried "i" could wrap around. Instead of making "i" an unsigned long (which may enlarge the type size), switch both nr_segs and i to u32. There is no truncation with nr_segs since it is never larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129183729.work.991-kees@kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>

The mix of int, unsigned int, and unsigned long used by struct poll_list::len, todo, len, and j meant that the signed overflow sanitizer got worried it needed to instrument several places where arithmetic happens between these variables. Since all of the variables are always positive and bounded by unsigned int, use a single type in all places. Additionally expand the zero-test into an explicit range check before updating "todo". This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: do_sys_poll+0x285: call to __ubsan_handle_sub_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Jan Kara <jack@suse.cz> Cc: <linux-fsdevel@vger.kernel.org> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129184014.work.593-kees@kernel.org Reviewed-by: Jan Kara <jack@suse.cz> Signed-off-by: Christian Brauner <brauner@kernel.org>

…_user() The loop counter "i" in copy_compat_iovec_from_user() is an int, but because the nr_segs argument is unsigned long, the signed overflow sanitizer got worried "i" could wrap around. Instead of making "i" an unsigned long (which may enlarge the type size), switch both nr_segs and i to u32. There is no truncation with nr_segs since it is never larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129183729.work.991-kees@kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>

The mix of int, unsigned int, and unsigned long used by struct poll_list::len, todo, len, and j meant that the signed overflow sanitizer got worried it needed to instrument several places where arithmetic happens between these variables. Since all of the variables are always positive and bounded by unsigned int, use a single type in all places. Additionally expand the zero-test into an explicit range check before updating "todo". This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: do_sys_poll+0x285: call to __ubsan_handle_sub_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Jan Kara <jack@suse.cz> Cc: <linux-fsdevel@vger.kernel.org> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129184014.work.593-kees@kernel.org Reviewed-by: Jan Kara <jack@suse.cz> Signed-off-by: Christian Brauner <brauner@kernel.org>

…_user() The loop counter "i" in copy_compat_iovec_from_user() is an int, but because the nr_segs argument is unsigned long, the signed overflow sanitizer got worried "i" could wrap around. Instead of making "i" an unsigned long (which may enlarge the type size), switch both nr_segs and i to u32. There is no truncation with nr_segs since it is never larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled Link: KSPP#26 [1] Link: https://lkml.kernel.org/r/20240129183729.work.991-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31a ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP". To annotate intentional wrap-around arithmetic, the add/sub/mul_wrap() helpers can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_WRAP_SIGNED_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_WRAP_SIGNED := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: KSPP/linux#26 [1] Link: https://lkml.kernel.org/r/20240205093725.make.582-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Marco Elver <elver@google.com> Cc: Justin Stitt <justinstitt@google.com> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nicolas Schier <nicolas@fjasle.eu> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

…_user() The loop counter "i" in copy_compat_iovec_from_user() is an int, but because the nr_segs argument is unsigned long, the signed overflow sanitizer got worried "i" could wrap around. Instead of making "i" an unsigned long (which may enlarge the type size), switch both nr_segs and i to u32. There is no truncation with nr_segs since it is never larger than UIO_MAXIOV anyway. This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: copy_compat_iovec_from_user+0xa9: call to __ubsan_handle_add_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129183729.work.991-kees@kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>

The mix of int, unsigned int, and unsigned long used by struct poll_list::len, todo, len, and j meant that the signed overflow sanitizer got worried it needed to instrument several places where arithmetic happens between these variables. Since all of the variables are always positive and bounded by unsigned int, use a single type in all places. Additionally expand the zero-test into an explicit range check before updating "todo". This keeps sanitizer instrumentation[1] out of a UACCESS path: vmlinux.o: warning: objtool: do_sys_poll+0x285: call to __ubsan_handle_sub_overflow() with UACCESS enabled Link: KSPP/linux#26 [1] Cc: Christian Brauner <brauner@kernel.org> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Jan Kara <jack@suse.cz> Cc: <linux-fsdevel@vger.kernel.org> Signed-off-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20240129184014.work.593-kees@kernel.org Reviewed-by: Jan Kara <jack@suse.cz> Signed-off-by: Christian Brauner <brauner@kernel.org>

…82432) Clang has a `signed-integer-overflow` sanitizer to catch arithmetic overflow; however, most of its instrumentation [fails to apply](https://godbolt.org/z/ee41rE8o6) when `-fwrapv` is enabled; this is by design. The Linux kernel enables `-fno-strict-overflow` which implies `-fwrapv`. This means we are [currently unable to detect signed-integer wrap-around](KSPP/linux#26). All the while, the root cause of many security vulnerabilities in the Linux kernel is [arithmetic overflow](https://cwe.mitre.org/data/definitions/190.html). To work around this and enhance the functionality of `-fsanitize=signed-integer-overflow`, we instrument signed arithmetic even if the signed overflow behavior is defined. Co-authored-by: Justin Stitt <justinstitt@google.com>

In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31a ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP". To annotate intentional wrap-around arithmetic, the add/sub/mul_wrap() helpers can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_WRAP_SIGNED_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_WRAP_SIGNED := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: KSPP/linux#26 [1] Link: https://lkml.kernel.org/r/20240205093725.make.582-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Marco Elver <elver@google.com> Cc: Justin Stitt <justinstitt@google.com> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nicolas Schier <nicolas@fjasle.eu> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31a ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP". To annotate intentional wrap-around arithmetic, the add/sub/mul_wrap() helpers can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_WRAP_SIGNED_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_WRAP_SIGNED := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: KSPP#26 [1] Link: https://lkml.kernel.org/r/20240205093725.make.582-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Marco Elver <elver@google.com> Cc: Justin Stitt <justinstitt@google.com> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nicolas Schier <nicolas@fjasle.eu> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31a ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP". To annotate intentional wrap-around arithmetic, the add/sub/mul_wrap() helpers can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_WRAP_SIGNED_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_WRAP_SIGNED := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: KSPP/linux#26 [1] Link: https://lkml.kernel.org/r/20240205093725.make.582-kees@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Marco Elver <elver@google.com> Cc: Justin Stitt <justinstitt@google.com> Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nicolas Schier <nicolas@fjasle.eu> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

In order to mitigate unexpected signed wrap-around[1], bring back the signed integer overflow sanitizer. It was removed in commit 6aaa31a ("ubsan: remove overflow checks") because it was effectively a no-op when combined with -fno-strict-overflow (which correctly changes signed overflow from being "undefined" to being explicitly "wrap around"). Compilers are adjusting their sanitizers to trap wrap-around and to detecting common code patterns that should not be instrumented (e.g. "var + offset < var"). Prepare for this and explicitly rename the option from "OVERFLOW" to "WRAP" to more accurately describe the behavior. To annotate intentional wrap-around arithmetic, the helpers wrapping_add/sub/mul_wrap() can be used for individual statements. At the function level, the __signed_wrap attribute can be used to mark an entire function as expecting its signed arithmetic to wrap around. For a single object file the Makefile can use "UBSAN_SIGNED_WRAP_target.o := n" to mark it as wrapping, and for an entire directory, "UBSAN_SIGNED_WRAP := n" can be used. Additionally keep these disabled under CONFIG_COMPILE_TEST for now. Link: KSPP/linux#26 [1] Cc: Miguel Ojeda <ojeda@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Hao Luo <haoluo@google.com> Reviewed-by: Marco Elver <elver@google.com> Reviewed-by: Justin Stitt <justinstitt@google.com> Signed-off-by: Kees Cook <keescook@chromium.org>

JustinStitt · 2024-02-27T00:24:49Z

Just dumping some info in this thread.

Here's how I currently see the progress on this issue as of now:

(note that these commit SHAs mentioned are from Kees' tree linked below and may change under force push / rebase circumstances).

make signed integer sanitizer work even with -fwrapv (and -fno-strict-overflow).
Sanitizer: Support -fwrapv with -fsanitize=signed-integer-overflow llvm/llvm-project#82432

create "expected signed overflow" helper inline functions marked with __attribute__((no_sanitize("signed-integer-overflow")))..

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=dev/v6.8-rc2/signed-overflow-sanitizer

with Commit e30a6c0 ("overflow: Introduce wrapping_add(), wrapping_sub(), and wrapping_mul()") and Commit 87e7e79 ("overflow: Introduce wrapping_assign_add() and wrapping_assign_sub()")

find all true positives and replace with helper calls.

"all" is a big word. hard to mark this as done.

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=dev/v6.8-rc2/signed-overflow-sanitizer

main offenders: refcount and atomics covered in tree with commits:

4d6a54c480d91c0c9210d1041342a38ac840d4a7
b037d9207c40cdae50424de2d9281bcfde9f3485
53003a74bdc005a4517de0ab0edca70ba28b0ed
1ebd18c24a127e5d7c56f9d888e2b3ad8c8e9f32

add note to "deprecated.rst" with something like "open coded signed integer wrap around without a helper".
^ todo

add back signed integer overflow as a UBSan Kconfig

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=dev/v6.8-rc2/signed-overflow-sanitizer

Done with 557f8c5

kees · 2024-02-27T02:17:26Z

Yes! I've updated the top-level now (and added you to the project ACLs to edit this for yourself in the future).

In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Switch to a more regular type for a 64-bit value and refactor the open-coded wrap-around addition test to use subtraction from the type max (since add_would_overflow() may not be defined in early boot code). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: KSPP#26 [2] Link: KSPP#27 [3] Link: KSPP#344 [4] Cc: Nick Terrell <terrelln@fb.com> Cc: Paul Jones <paul@pauljones.id.au> Cc: Sedat Dilek <sedat.dilek@gmail.com> Cc: Oleksandr Natalenko <oleksandr@natalenko.name> Cc: Xin Gao <gaoxin@cdjrlc.com> Signed-off-by: Kees Cook <keescook@chromium.org>

In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Switch to a more regular type for a 64-bit value and refactor the open-coded wrap-around addition test to use subtraction from the type max (since add_would_overflow() may not be defined in early boot code). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: KSPP/linux#26 [2] Link: KSPP/linux#27 [3] Link: KSPP/linux#344 [4] Cc: Nick Terrell <terrelln@fb.com> Cc: Paul Jones <paul@pauljones.id.au> Cc: Sedat Dilek <sedat.dilek@gmail.com> Cc: Oleksandr Natalenko <oleksandr@natalenko.name> Cc: Xin Gao <gaoxin@cdjrlc.com> Signed-off-by: Kees Cook <keescook@chromium.org>

In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Switch to a more regular type for a 64-bit value and refactor the open-coded wrap-around addition test to use subtraction from the type max (since add_would_overflow() may not be defined in early boot code). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: KSPP#26 [2] Link: KSPP#27 [3] Link: KSPP#344 [4] Cc: Nick Terrell <terrelln@fb.com> Cc: Paul Jones <paul@pauljones.id.au> Cc: Sedat Dilek <sedat.dilek@gmail.com> Cc: Oleksandr Natalenko <oleksandr@natalenko.name> Cc: Xin Gao <gaoxin@cdjrlc.com> Signed-off-by: Kees Cook <keescook@chromium.org>

kees mentioned this issue Dec 5, 2019

Detect unsigned integer wrap-around (overflow and underflow) #27

Open

6 tasks

kees added the compiler Needs compiler support label Dec 5, 2019

kees mentioned this issue Aug 26, 2020

syzbot: enable UBSAN for linux google/syzkaller#1523

Closed

This was referenced Sep 15, 2023

Detect pointer math wrap-around (overflow and underflow) #344

Open

Mitigate arithmetic wrap-around (overflow and underflow) #345

Open

kees changed the title ~~Detect signed integer overflows~~ Detect signed integer wrap-around (overflow and underflow) Sep 25, 2023

JustinStitt mentioned this issue Jan 31, 2024

[Sanitizer] add signed-integer-wrap sanitizer llvm/llvm-project#80089

Closed

JustinStitt mentioned this issue Feb 20, 2024

Sanitizer: Support -fwrapv with -fsanitize=signed-integer-overflow llvm/llvm-project#82432

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect signed integer wrap-around (overflow and underflow) #26

Detect signed integer wrap-around (overflow and underflow) #26

kees commented Dec 5, 2019 •

edited

Loading

kees commented Aug 12, 2021

kees commented Sep 2, 2021

kees commented Sep 14, 2021

kees commented Sep 14, 2021

kees commented Sep 25, 2021

kees commented Jan 28, 2022

bwendling commented Sep 20, 2023 •

edited

Loading

kees commented Sep 25, 2023

JustinStitt commented Jan 8, 2024 •

edited

Loading

kees commented Jan 8, 2024

kees commented Jan 9, 2024

JustinStitt commented Jan 31, 2024

JustinStitt commented Feb 27, 2024 •

edited

Loading

kees commented Feb 27, 2024

Detect signed integer wrap-around (overflow and underflow) #26

Detect signed integer wrap-around (overflow and underflow) #26

Comments

kees commented Dec 5, 2019 • edited Loading

kees commented Aug 12, 2021

kees commented Sep 2, 2021

kees commented Sep 14, 2021

kees commented Sep 14, 2021

kees commented Sep 25, 2021

kees commented Jan 28, 2022

bwendling commented Sep 20, 2023 • edited Loading

kees commented Sep 25, 2023

JustinStitt commented Jan 8, 2024 • edited Loading

kees commented Jan 8, 2024

kees commented Jan 9, 2024

JustinStitt commented Jan 31, 2024

JustinStitt commented Feb 27, 2024 • edited Loading

kees commented Feb 27, 2024

kees commented Dec 5, 2019 •

edited

Loading

bwendling commented Sep 20, 2023 •

edited

Loading

JustinStitt commented Jan 8, 2024 •

edited

Loading

JustinStitt commented Feb 27, 2024 •

edited

Loading