From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by sourceware.org (Postfix) with ESMTPS id 22AAD3858C2B for ; Mon, 18 Sep 2023 07:32:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 22AAD3858C2B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x135.google.com with SMTP id 2adb3069b0e04-50307759b65so2307357e87.0 for ; Mon, 18 Sep 2023 00:32:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695022319; x=1695627119; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=DbLLUgzi+hnFZAyZM3oaTuZ1LqlcJfihjwYs90E/US0=; b=SHwNrhP9JcMgu62bvWxQB21jGC/Ib9H34NPBAE9SN8scCU1cCjxshBV3CosahA9Yb1 SVMbKDUKwV1/I4+g++CNlVRaXsOdMQC9SlIqxrtBp6VdPnvQS5qFGfpXGtK3Xe1VWRnA 5LlKP0pFBsT8K7J2dssBbkcOa+6J9UCnbQ//uirP57Jr7EQpFAfIy6ekJasZTxfc2ioy JZRaQu9ITalaDQxPtQuy6fD5S8rkWRtgSZel1W6EwZ4Wbi8HZNqagWvxOM3UUTUj13Fs 8alm+IP/cgJ1lc09uhUyFWILhh8zvh3ZJoskgDfxMi5YBZATwbvOEwojXbKmVEMfnXVK dAKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695022319; x=1695627119; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DbLLUgzi+hnFZAyZM3oaTuZ1LqlcJfihjwYs90E/US0=; b=tN3v8/qMerF1s1medP3lxAjW28hW+BAtCZX6agGhRFzWotOhLZuJyXCRsCpTsSiYeQ wIYrk+1G0jhHCnoOKFB+DVC7ZePKLUPwpoiw388R8atgCU1pqjMi/Ao8BTMR1LXN+ptt RgJNpU8ddiQiIs6WD89GKmS9SS2as6DEvQSYDV9EoWUwtAzEZkv7T3Oe1AOiA61UcGD2 8fj0/Tu8w0MoaVCEDn5SePYwQz3+OjX7ZKZiyquT57zSW0w/aSU7ss7HTz4ybJcvGnYa 4G1Zj8ZOsaE44qOB24TKTu0KtbOeLOugFm3bmu+RuJ47tg6Dsn21m8w7EfbDpIPRkm8V tTJw== X-Gm-Message-State: AOJu0YyG+FS2ox+o5jVozNTqjZSRJ3PeDzXCft/uWTeJp9+PWeHdgnfa AG6FaORgJZNfYzBpvp13Tzpwy3SxICZLkWPR3Sk= X-Google-Smtp-Source: AGHT+IFpDO7fj+ba936rSDab+EoJpMK8ch6MkBY0zZGOLlOwvtscmfJOxtwrz0hbOHzTqom9qzKO4jEP8Rysc5S5b2c= X-Received: by 2002:ac2:4831:0:b0:4fe:8c1d:9e81 with SMTP id 17-20020ac24831000000b004fe8c1d9e81mr6173511lft.36.1695022319346; Mon, 18 Sep 2023 00:31:59 -0700 (PDT) MIME-Version: 1.0 References: <202309151107.C2F7D9A01@keescook> <228be10b14dabe4e8216f9f93da99f629328b3e0.camel@gmail.com> In-Reply-To: <228be10b14dabe4e8216f9f93da99f629328b3e0.camel@gmail.com> From: Richard Biener Date: Mon, 18 Sep 2023 09:31:47 +0200 Message-ID: Subject: Re: Question on -fwrapv and -fwrapv-pointer To: Martin Uecker Cc: keescook@chromium.org, gcc@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Sat, Sep 16, 2023 at 10:38=E2=80=AFAM Martin Uecker via Gcc wrote: > > > > (moved to gcc@) > > > On Fri, Sep 15, 2023 at 08:18:28AM -0700, Andrew Pinski wrote: > > > On Fri, Sep 15, 2023 at 8:12=E2=80=AFAM Qing Zhao wrote: > > > > > > > > > > > > > > > > > On Sep 15, 2023, at 3:43 AM, Xi Ruoyao wrote= : > > > > > > > > > > On Thu, 2023-09-14 at 21:41 +0000, Qing Zhao wrote: > > > > >>>> CLANG already provided -fsanitize=3Dunsigned-integer-overflow.= GCC > > > > >>>> might need to do the same. > > > > >>> > > > > >>> NO. There is no such thing as unsigned integer overflow. That o= ption > > > > >>> is badly designed and the GCC community has rejected a few time= s now > > > > >>> having that sanitizer before. It is bad form to have a sanitize= r for > > > > >>> well defined code. > > > > >> > > > > >> Even though unsigned integer overflow is well defined, it might = be > > > > >> unintentional, shall we warn user about this? > > > > > > > > > > *Everything* could be unintentional and should be warned then. G= CC is a > > > > > compiler, not an advanced AI educating the programmers. > > > > > > > > Well, you are right in some sense. -:) > > > > > > > > However, overflow is one important source for security flaws, it=E2= =80=99s important for compilers to detect > > > > overflows in the programs in general. > > > > > > Except it is NOT an overflow. Rather it is wrapping. That is a big > > > point here. unsigned wraps and does NOT overflow. Yes there is a majo= r > > > difference. > > > > Right, yes. I will try to pick my language very carefully. :) > > > > The practical problem I am trying to solve in the 30 million lines of > > Linux kernel code is that of catching arithmetic wrap-around. The > > problem is one of evolving the code -- I can't just drop -fwrapv and > > -fwrapv-pointer because it's not possible to fix all the cases at once. > > (And we really don't want to reintroduce undefined behavior.) > > > > So, for signed, pointer, and unsigned types, we need: > > > > a) No arithmetic UB -- everything needs to have deterministic behavior. > > The current solution here is "-fno-strict-overflow", which eliminate= s > > the UB and makes sure everything wraps. > > > > b) A way to run-time warn/trap on overflow/underflow/wrap-around. This > > would work with -fsanitize=3D[signed-integer|pointer]-overflow excep= t > > due to "a)" we always wrap. And there isn't currently coverage like > > this for unsigned (in GCC). > > > > Our problem is that the kernel is filled with a mix of places where the= re > > is intended wrap-around and unintended wrap-around. We can chip away at > > fixing the intended wrap-around that we can find with static analyzers, > > etc, but at the end of the day there is a long tail of finding the plac= es > > where intended wrap-around is hiding. But when the refactoring is > > sufficiently completely, we can move the wrap-around warning to a trap, > > and the kernel will not longer have this class of security flaw. > > > > As a real-world example, here is a bug where a u8 wraps around causing > > an under-allocation that allowed for a heap overwrite: > > > > https://git.kernel.org/linus/6311071a0562 > > https://elixir.bootlin.com/linux/v6.5/source/net/wireless/nl80211.c#L54= 22 > > > > If there were more than 255 elements in a linked list, the allocation > > would be too small, and the second loop would write past the end of the > > allocation. This is a pretty classic allocation underflow and linear > > heap write overflow security flaw. (And it would be trivially stopped b= y > > trapping on the u8 wrap around.) > > > > So, I want to be able to catch that at run-time. But we also have code > > doing things like "if (ulong + offset < ulong) { ... }": > > > > https://elixir.bootlin.com/linux/v6.5/source/drivers/crypto/axis/artpec= 6_crypto.c#L1187 > > > > This is easy for a static analyzer to find and we can replace it with a > > non-wrapping test (e.g. __builtin_add_overflow()), but we'll not find > > them all immediately, especially for the signed and pointer cases. > > > > So, I need to retain the "everything wraps" behavior while still being > > able to detect when it happens. > > > Hi Kees, > > I have a couple of questions: > > Currently, my thinking was that you would use signed integers > if you want the usual integer arithmetic rules we know from > elementary school and if you overflow this is clearly a bug > you can diagnose with UBsan. > > There are people who think that signed overflow should be > defined to wrap, but I think this would be a severe > mistake because then code would start to rely on it, which > makes it then difficult to differentiate between bugs and > intended uses (e.g. the unfortunate situation you have > with the kernel). > > I assume you want to combine UBSan plus wrapping for > production use? Or only for testing? Or in other words: > why would testing UBSan and production with wrapping > not be sufficient to find and fix all bugs? > > Wrapping would not be correct because it may lead to > logic errors or use-after-free etc. I assume it is still > preferred because it more deterministic than whatever comes > out of the optimizer assuming that overflow has UB. Is this > the reasoning applied here? > > > For unsigned the intended use case is modulo arithmetic > where wrapping is the correct behavior. At least, this > is what I thought so far.. This seems also to be the > position of the overall GCC community rejecting > -fsanitize=3Dunsigned-integer-overflow. > > But then there are also people like Richard Seacord that > have the position that one should use "unsigned" for > every quantity which can not be negative, which implies > that then the modulo use case becomes the exception. > > Related to this I have the following question: In the > bug you refer to above, an unsigned variable was used > for something that is not meant for modulo arithmetic. > Is this done in the kernel to save space? I.e. because > 127 would not be enough as maximum but going to i16 takes > to much space? or is this for compatibility with some > on-the-wire protocol? > > Would an attribute for variables help that tells the > compiler that if stores to the variable wrap around > then this is not intended and this is an error? > > u8 x [[nowrap]]; > > x =3D 256; // can be diagnosed ? There's a problem with representing this, it's a long-standing thing standi= ng in the way of handling -fwrapv in the IL. Maybe C wants to have 'nonnegative int' in addition to 'unsigned int', or 'bit int' for the "just some two-complement bits". For GCC we think of -fwrapv to perform signed integer arithmetic as if promoted to a large enough type first and then truncated to the original type in the implementation defined manner (modulo reducing). But we end up doing what the actual hardware does (I'm for example unsure how that handles INT_MIN / -1 - x86 documents it as raising #DE which it indeed does, even with -fwrapv). volatile int a, b; int main () { a =3D -__INT_MAX__ - 1; b =3D -1; int z =3D a / b; __builtin_printf ("%d\n", z); } raises SIGFPE with and without -fwrapv. I'll note that we document -fwrapv to only affect addition, subtraction and multiplication (it also affects negation), matching the libgcc *v functions we have for -ftrapv. -fsanitize=3Dundefined instruments division but -fsanitize=3Drecover doesn't help. Maybe there's a bug about this recorded. The documentation of -fwrapv should likely be clarified. For recent work I failed to spot the C standards language on singed overflow behavior for negation, addition, subtraction and multiplication. I can find it for division though. Can someone point me to where that is (moved?)? Richard. > > > > Martin > > > > > >