Message-ID: <CAFP8O3Jq9kxx8xp4hK42+FwL3KnibDqpke5S7zT40kwmaEfXcA@mail.gmail.com>
Date: Fri, 17 May 2024 15:04:21 -0700
From: Fangrui Song <maskray@...gle.com>
To: Justin Stitt <justinstitt@...gle.com>, Peter Zijlstra <peterz@...radead.org>
Cc: Kees Cook <kees@...nel.org>, Linus Torvalds <torvalds@...ux-foundation.org>, 
	Kees Cook <keescook@...omium.org>, Mark Rutland <mark.rutland@....com>, 
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org, 
	llvm@...ts.linux.dev
Subject: Re: [RFC] Mitigating unexpected arithmetic overflow

On Thu, May 16, 2024 at 12:49 PM Justin Stitt <justinstitt@...gle.com> wrote:
>
> Hi,
>
> On Thu, May 16, 2024 at 7:09 AM Peter Zijlstra <peterz@...radead.org> wrote:
> >
> > On Thu, May 16, 2024 at 06:30:32AM -0700, Kees Cook wrote:
> > >
> > > I am a broken record. :) This is _not_ about undefined behavior.
> >
> > And yet you introduced CONFIG_UBSAN_SIGNED_WRAP... *UB*san, get it?
>
> We should think of UBSAN as an "Unexpected Behavior" Sanitizer. Clang
> has evolved to provide instrumentation for things that are not
> *technically* undefined behavior.
>
> Go to [1] and grep for some phrases like "not undefined behavior" and
> see that we have quite a few sanitizers that aren't *technically*
> handling undefined behavior.
>
> >
> > > This is about finding a way to make the intent of C authors
> > > unambiguous. That overflow wraps is well defined. It is not always
> > > _desired_. C has no way to distinguish between the two cases.
> >
> > The current semantics are (and have been for years, decades at this
> > point) that everything wraps nicely and code has been assuming this. You
> > cannot just change this.
>
> Why not :>)
>
> Lots and lots of exploits are caused by unintentional arithmetic overflow [2].
>
> >
> > So what you do is do a proper language extension and add a type
> > qualifier that makes overflows trap and annotate all them cases where
> > people do not expect overflows (so that we can put the
> > __builtin_*_overflow() things where the sun don't shine).
>
> It is incredibly important that the exact opposite approach is taken;
> we need to be annotating (or adding type qualifiers to) the _expected_
> overflow cases. The omniscience required to go and properly annotate
> all the spots that will cause problems would suggest that we should
> just fix the bug outright. If only it was that easy.
>
> I don't think we're capable of identifying every single problematic
> overflow/wraparound case in the kernel, this is pretty obvious
> considering we've had decades to do so. Instead, it seems much more
> feasible that we annotate (very, very minimally so as not to disrupt
> code readability and style) the spots where we _know_ overflow should
> happen.
>
> [1]: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#ubsan-checks
> [2]: https://cwe.mitre.org/data/definitions/190.html
>
> Thanks
> Justin

FWIW I have made -fsanitize=undefined -fwrapv not imply
-fsanitize=signed-integer-overflow in
https://github.com/llvm/llvm-project/pull/85501 .
The change is not included in the LLVM 18.1.* releases.
This might encourage projects using both -fwrapv and
-fsanitize=undefined to re-evaluate whether -fwrapv aligns with their
needs.

The description of #85501 contains more information about my
understanding of kernel security folks' mind:

> Linux kernel uses -fwrapv to change signed integer overflows from undefined behaviors to defined behaviors. However, the security folks still want -fsanitize=signed-integer-overflow diagnostics. Their intention can be expressed with -fwrapv -fsanitize=signed-integer-overflow (#80089). This mode by default reports recoverable errors while still making signed integer overflows defined (most UBSan checks are recoverable by default: you get errors in stderr, but the  program is not halted).


-- 
宋方睿

Powered by blists - more mailing lists