Justification of UBSAN-related choices?
equaeghe opened this issue · 5 comments
Currently, UBSAN-related choices are as follows:
It is unclear to me why the last two are chosen. UBSAN_MISC=y seems like a good thing, as it enables more checks. UBSAN_TRAP=y seems like a bad thing, as it enables denial of service attacks. Furthermore, if I understand things correctly, UBSAN_SANITIZE_ALL=y would be needed to practically activate UBSAN.
Is my understanding correct, or a misunderstanding (which is perfectly possible). In the latter case, I would be grateful for a pointer to an appropriate resource.
Hello @equaeghe
Thanks for your question.
Please have a look, @kees wrote about that in his article about security-related things in the Linux kernel 5.7:
https://outflux.net/blog/archives/2020/09/21/security-things-in-linux-v5-7/
Quote:
For runtime checking, the Undefined Behavior Sanitizer has an option for adding runtime array bounds checking
for catching things like this where the compiler cannot perform a static analysis of the index values.
...
It was, however, not separate (via kernel Kconfig) until Elena Petrova and I split it out into
CONFIG_UBSAN_BOUNDS, which is fast enough for production kernel use.
...
Since UBSAN (and the other Sanitizers) only WARN() by default, system owners need to
set panic_on_warn=1 too if they want to defend against attacks targeting these kinds of flaws.
Because of this, and to avoid bloating the kernel image with all the warning messages, I introduced
CONFIG_UBSAN_TRAP which effectively turns these conditions into a BUG() without needing
additional sysctl settings.
Does that provide answers to your questions?
Thanks, that explains why UBSAN_TRAP=y. I am still unclear why UBSAN_MISC is not set and why nothing is said about UBSAN_SANITIZE_ALL.
It looks like other UBSAN modes are for kernel debugging, not for hardening:
[*] Perform checking for bit-shift overflows
[*] Perform checking for integer divide-by-zero
[*] Perform checking for non-boolean values used as boolean
[*] Perform checking for out of bounds enum values
[*] Perform checking for misaligned pointer usage
Previously they were collected under UBSAN_MISC, but now I see that they are separate since the kernel commit c637693b20da8706b7f48d96882c9c80ae935151. I will have a closer look at them.
I will also test UBSAN_SANITIZE_ALL behavior.
Thanks @equaeghe !
UBSAN_SANITIZE_ALL is needed to gain coverage over the kernel as a whole. Otherwise, only opted-in things will have the UBSAN features applied.
I.e. for production workloads, I recommend:
CONFIG_UBSAN=y
CONFIG_UBSAN_BOUNDS=y
CONFIG_UBSAN_SANITIZE_ALL=y
and depending on one's crash tolerances, either use panic_on_warn=1 or CONFIG_UBSAN_TRAP=y.