LKML Archive mirror
 help / color / mirror / Atom feed
From: Martin Uecker <uecker@tugraz.at>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>,
	Justin Stitt <justinstitt@google.com>,
	 Peter Zijlstra <peterz@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-hardening@vger.kernel.org,  linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [RFC] Mitigating unexpected arithmetic overflow
Date: Sun, 12 May 2024 21:29:15 +0200	[thread overview]
Message-ID: <bf7539e0ccb8f445984fe6dab0d7d8392a79880d.camel@tugraz.at> (raw)
In-Reply-To: <CAHk-=wjERv03yFU7-RUuqX1y89DYHcpdsuu++ako2nR41-EjYg@mail.gmail.com>

Am Sonntag, dem 12.05.2024 um 09:09 -0700 schrieb Linus Torvalds:
> On Sun, 12 May 2024 at 01:03, Martin Uecker <uecker@tugraz.at> wrote:
> > 
> > But I guess it still could be smarter. Or does it have to be a
> > sanitizer because compile-time will always have too many false
> > positives?
> 
> Yes, there will be way too many false positives.
> 
> I'm pretty sure there will be a ton of "intentional positives" too,
> where we do drop bits, but it's very much intentional. I think
> somebody already mentioned the "store little endian" kind of things
> where code like
> 
>         unsigned chat *p;
>         u32 val;
> 
>         p[0] = val;
>         p[1] = val >> 8;
>         p[2] = val >> 16;
>         p[3] = val >> 24;
> 
> kind of code is both traditional and correct, but obviously drops bits
> very much intentionally on each of those assignments.
> 
> Now, obviously, in a perfect world the compiler would see the above as
> "not really dropping bits", but that's not the world we live in.
> 
> So the whole "cast drops bits" is not easy to deal with.
> 
> In the case of the above kind of byte-wise behavior, I do think that
> we could easily make the byte masking explicit, and so in *some* cases
> it might actually be a good thing to just make these things more
> explicit, and write it as
> 
>         p[0] = val & 0xff;
>         p[1] = (val >> 8) & 0xff;
>         ...
> 
> and the above doesn't make the source code worse: it arguably just
> makes things more explicit both for humans and for the compiler, with
> that explicit bitwise 'and' operation making it very clear that we're
> just picking a particular set of bits out of the value.

Adding versions of the -Wconversions warning which triggers only
in very specific cases should not be too difficult, if something
like this is useful, i.e. restricting the warning to assignments.

> 
> But I do suspect the "implicit cast truncates value" is _so_ common
> that it might be very very painful. Even with a run-time sanitizer
> check.
> 
> And statically I think it's entirely a lost cause - it's literally
> impossible to avoid in C. Why? Because there are no bitfield
> variables, only fields in structures/unions, so if you pass a value
> around as an argument, and then end up finally assigning it to a
> bitfield, there was literally no way to pass that value around as the
> "right type" originally. The final assignment *will* drop bits from a
> static compiler standpoint.
> 

If one wanted to, one could always pass bitfields inside a struct

typedef struct { unsigned int v:12; } b12;

int f(b12 x)
{
    int i = x.v;
    return i & (1 << 13);
}

the compiler is then smart enough to know how many bits are
relevant and track this to some degree inside the function.


https://godbolt.org/z/o8P3adnEK

But using this information for warnings would be more difficult
because the information is not computed in the front end. (but 
here also other warnings generated by the backend, so not
impossible). And, of course, the additional wrapping and
unwrapping makes the code more ugly (*)

C23 then also has bit-precise integers.

Martin

(*) ...but compared to what some other languages require the
programmer to write, even  this seems relatively benign.












  reply	other threads:[~2024-05-12 19:29 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-07 23:27 [RFC] Mitigating unexpected arithmetic overflow Kees Cook
2024-05-08 12:22 ` David Laight
2024-05-08 23:43   ` Kees Cook
2024-05-08 17:52 ` Linus Torvalds
2024-05-08 19:44   ` Kees Cook
2024-05-08 20:07     ` Linus Torvalds
2024-05-08 22:54       ` Kees Cook
2024-05-08 23:47         ` Linus Torvalds
2024-05-09  0:06           ` Linus Torvalds
2024-05-09  0:23           ` Linus Torvalds
2024-05-09  6:11           ` Kees Cook
2024-05-09 14:08             ` Theodore Ts'o
2024-05-09 15:38               ` Linus Torvalds
2024-05-09 17:54                 ` Al Viro
2024-05-09 18:08                   ` Linus Torvalds
2024-05-09 18:39                     ` Linus Torvalds
2024-05-09 18:48                       ` Al Viro
2024-05-09 19:15                         ` Linus Torvalds
2024-05-09 19:28                           ` Al Viro
2024-05-09 21:06                 ` David Laight
2024-05-18  5:11             ` Matthew Wilcox
2024-05-09 21:23           ` David Laight
2024-05-12  8:03           ` Martin Uecker
2024-05-12 16:09             ` Linus Torvalds
2024-05-12 19:29               ` Martin Uecker [this message]
2024-05-13 18:34               ` Kees Cook
2024-05-15  7:36           ` Peter Zijlstra
2024-05-15 17:12             ` Justin Stitt
2024-05-16  7:45               ` Peter Zijlstra
2024-05-16 13:30             ` Kees Cook
2024-05-16 14:09               ` Peter Zijlstra
2024-05-16 19:48                 ` Justin Stitt
2024-05-16 20:07                   ` Kees Cook
2024-05-16 20:51                   ` Theodore Ts'o
2024-05-17 21:15                     ` Kees Cook
2024-05-18  2:51                       ` Theodore Ts'o
2024-05-17 22:04                   ` Fangrui Song
2024-05-18 13:08               ` David Laight
2024-05-15  7:57           ` Peter Zijlstra
2024-05-17  7:45       ` Jonas Oberhauser
2024-05-11 16:19 ` Dan Carpenter
2024-05-13 19:43   ` Kees Cook
2024-05-14  8:45     ` Dan Carpenter
2024-05-18 15:39       ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bf7539e0ccb8f445984fe6dab0d7d8392a79880d.camel@tugraz.at \
    --to=uecker@tugraz.at \
    --cc=justinstitt@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).