Re: [PATCH v3] ubsan: Reintroduce signed overflow sanitizer

Kees Cook
2024-02-07T11:04:16Z

On Wed, Feb 07, 2024 at 01:45:28AM +0000, Justin Stitt wrote:
> I wouldn't mind also seeing a test_ubsan_div_overflow test case here.
>
> It has some quirky behavior and it'd be nice to test that the sanitizers
> properly capture it.
>
> Check out this Godbolt: https://godbolt.org/z/qG5f1j6n1
>
> tl;dr: with -fsanitize=signed-integer-overflow division (/) and
> remainder (%) operators still instrument arithmetic even with
> -fno-strict-overflow on.
>
> This makes sense as division by 0 and INT_MIN/-1 are UBs that are not
> influenced by -fno-strict-overflow.

There is actually already a test_ubsan_divrem_overflow, but because the
failure modes result in a trap even without the sanitizer, it's disabled
in the test. For testing a crashing mode, it might be interesting to add
it to LKDTM, which is the crash tester...

>
> Really though, the patch is fine and the above test case is optional and
> can be shipped later -- as such:
>
> Reviewed-by: Justin Stitt <justinstitt@google.com>

Thanks!

-Kees

--
Kees Cook