[Bug sanitizer/94307] New: Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[Bug sanitizer/94307] New: Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

            Bug ID: 94307
           Summary: Provide a way to declare the *SAN exception handler
                    -fsanitize-undefined-trap-on-error
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kees at outflux dot net
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---

Instead of unconditionally calling __builtin_trap() for
-fsanitize-undefined-trap-on-error it would help the Linux kernel's use of
UBSAN to have a way to specify the trap function. With that, Linux can use its
own internal exception handling routines and avoid various confused states:

https://lore.kernel.org/linux-next/20200324164433.qusyu5h7ykx3f2bu@treble/

For example something like -fsanitize-undefined-trap-function=__ubsan_trap and
"__ubsan_trap" can then be defined by the kernel itself. Using the standard
handler routines (__ubsan_handle_*) are too heavy duty for some builds, so a
regular trap is needed for the kernel, but this allows us to provide a
"continue anyway" option as well.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
But isn't the default without -fsanitize-undefined-trap-on-error already
calling
a library routine that the kernel could override?

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-03-25
   Target Milestone|---                         |11.0
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Kees Cook from comment #0)
> Instead of unconditionally calling __builtin_trap() for
> -fsanitize-undefined-trap-on-error it would help the Linux kernel's use of
> UBSAN to have a way to specify the trap function. With that, Linux can use
> its own internal exception handling routines and avoid various confused
> states:

Sure, that's definitely possible.

> 
> https://lore.kernel.org/linux-next/20200324164433.qusyu5h7ykx3f2bu@treble/
> 
> For example something like -fsanitize-undefined-trap-function=__ubsan_trap
> and "__ubsan_trap" can then be defined by the kernel itself. Using the
> standard handler routines (__ubsan_handle_*) are too heavy duty for some
> builds, so a regular trap is needed for the kernel, but this allows us to
> provide a "continue anyway" option as well.

I would rather add something like
-fsanitize-undefined-handler=[runtime,trap,handler] where
- runtime would call current __ubsan_handle_*
- trap would result in ud2
- handler - can be call to __ubsan_trap

Where I can imagine it will call 2 versions (__ubsan_trap and
__ubsan_trap_no_return). That will depend on -fsanitize-recovery=..

I can do a patch for GCC 11.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> But isn't the default without -fsanitize-undefined-trap-on-error already
> calling
> a library routine that the kernel could override?

They don't want to implement all the:
        call    __ubsan_handle_add_overflow
        call    __ubsan_handle_divrem_overflow
        call    __ubsan_handle_dynamic_type_cache_miss
        call    __ubsan_handle_invalid_builtin
        call    __ubsan_handle_load_invalid_value
        call    __ubsan_handle_mul_overflow
        call    __ubsan_handle_negate_overflow
        call    __ubsan_handle_nonnull_arg
        call    __ubsan_handle_out_of_bounds
        call    __ubsan_handle_pointer_overflow
        call    __ubsan_handle_shift_out_of_bounds
        call    __ubsan_handle_sub_overflow
        call    __ubsan_handle_type_mismatch_v1
...
run-time entry points.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, they could just make them alias of each other, that is not the big deal,
I guess they don't want to waste .rodata space on the data that provides the
details to those functions and waste .text on passing the addresses of such
data to the handlers.  Of course, it would be much harder to understand what is
going on when the functions don't tell you what exactly went wrong (but that is
the same with -fsanitize-undefined-trap-on-error).

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #5 from Kees Cook <kees at outflux dot net> ---
Hi! I recently learned that Clang has -fsanitizer-minimal-runtime that is very
close to what I was expecting to use:

https://bugs.llvm.org/show_bug.cgi?id=45295

That is close to what you're already suggesting. Would it be possible to do the
same thing? That way the kernel can have just one "not the full debug details"
handler.

Thanks for looking at this!

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Kees Cook from comment #5)
> Hi! I recently learned that Clang has -fsanitizer-minimal-runtime that is
> very close to what I was expecting to use:
> 
> https://bugs.llvm.org/show_bug.cgi?id=45295

Looking at the implementation, you'll still have to implement ~50 entry points:
#define HANDLER_RECOVER(name, msg)                               \
  INTERFACE void __ubsan_handle_##name##_minimal() {             \
    if (!report_this_error(__builtin_return_address(0))) return; \
    message("ubsan: " msg "\n");                                 \
  }

#define HANDLER_NORECOVER(name, msg)                             \
  INTERFACE void __ubsan_handle_##name##_minimal_abort() {       \
    message("ubsan: " msg "\n");                                 \
    abort_with_message("ubsan: " msg);                           \
  }

#define HANDLER(name, msg)                                       \
  HANDLER_RECOVER(name, msg)                                     \
  HANDLER_NORECOVER(name, msg)

HANDLER(type_mismatch, "type-mismatch")
HANDLER(alignment_assumption, "alignment-assumption")
HANDLER(add_overflow, "add-overflow")
HANDLER(sub_overflow, "sub-overflow")
HANDLER(mul_overflow, "mul-overflow")
HANDLER(negate_overflow, "negate-overflow")
HANDLER(divrem_overflow, "divrem-overflow")
HANDLER(shift_out_of_bounds, "shift-out-of-bounds")
HANDLER(out_of_bounds, "out-of-bounds")
HANDLER_RECOVER(builtin_unreachable, "builtin-unreachable")
HANDLER_RECOVER(missing_return, "missing-return")
HANDLER(vla_bound_not_positive, "vla-bound-not-positive")
HANDLER(float_cast_overflow, "float-cast-overflow")
HANDLER(load_invalid_value, "load-invalid-value")
HANDLER(invalid_builtin, "invalid-builtin")
HANDLER(function_type_mismatch, "function-type-mismatch")
HANDLER(implicit_conversion, "implicit-conversion")
HANDLER(nonnull_arg, "nonnull-arg")
HANDLER(nonnull_return, "nonnull-return")
HANDLER(nullability_arg, "nullability-arg")
HANDLER(nullability_return, "nullability-return")
HANDLER(pointer_overflow, "pointer-overflow")
HANDLER(cfi_check_fail, "cfi-check-fail")

> 
> That is close to what you're already suggesting. Would it be possible to do
> the same thing? That way the kernel can have just one "not the full debug
> details" handler.

Well, it can be possible to implement the same. But I would like to see first a
kernel discussion to happen. You can prepare a patch that will utilize clang
and their -fsanitizer-minimal-runtime and sent it to Kernel mailing list. Would
it be possible?

> 
> Thanks for looking at this!

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 48209
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48209&action=edit
Patch candidate for shift_out_of_bounds

Patch for GCC that supports -fsanitize-minimal-runtime for shift_out_of_bounds,
but minimal UBSAN run-time library is not provided.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
@Kees: Any update on kernel side?

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
Any update on this Kees?

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|marxin at gcc dot gnu.org          |unassigned at gcc dot gnu.org
             Status|ASSIGNED                    |NEW

--- Comment #10 from Martin Liška <marxin at gcc dot gnu.org> ---
Apparently there's not much interest. Leaving as unassigned.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.0                        |11.2

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.1 has been released, retargeting bugs to GCC 11.2.

[Bug sanitizer/94307] Provide a way to declare the *SAN exception handler -fsanitize-undefined-trap-on-error

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94307

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.2                        |---