From: Dan Carpenter
> Sent: 14 May 2024 09:45
>
> Snipped all the bits where you are clearly correct.
>
> On Mon, May 13, 2024 at 12:43:37PM -0700, Kees Cook wrote:
> > > drivers/usb/class/usbtmc.c:852 usbtmc_generic_read() warn: potential
> > > integer overflow from user
> 'max_transfer_size +
From: Kees Cook
> Sent: 16 May 2024 14:31
>
> On May 15, 2024 12:36:36 AM PDT, Peter Zijlstra wrote:
> >On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> >> For example, the most common case of overflow we've ever had has very
> >> much been array indexing. Now, sometimes that has
On Wed, May 08, 2024 at 11:11:35PM -0700, Kees Cook wrote:
> Or even just simple math between u8s:
>
> while (xas->xa_offset == 255) {
> xas->xa_offset = xas->xa_node->offset - 1;
>
> Is it expecting to wrap (truncate)? How is it distinguished from
> the "num
On Fri, May 17, 2024 at 02:15:01PM -0700, Kees Cook wrote:
> On Thu, May 16, 2024 at 02:51:34PM -0600, Theodore Ts'o wrote:
> > On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote:
> > >
> > > It is incredibly important that the exact opposite approach is taken;
> > > we need to be annota
On Thu, May 16, 2024 at 12:49 PM Justin Stitt wrote:
>
> Hi,
>
> On Thu, May 16, 2024 at 7:09 AM Peter Zijlstra 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 CON
On Thu, May 16, 2024 at 02:51:34PM -0600, Theodore Ts'o wrote:
> On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote:
> >
> > 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
Am 5/8/2024 um 10:07 PM schrieb Linus Torvalds:
And no, the answer is ABSOLUTELY NOT to add cognitive load on kernel
developers by adding yet more random helper types and/or functions.
Just to show an option without "more types and helper functions", one
could also instead add a coverage r
On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote:
>
> 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 th
On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote:
> 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 (ver
Hi,
On Thu, May 16, 2024 at 7:09 AM Peter Zijlstra 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
On Thu, May 16, 2024 at 06:30:32AM -0700, Kees Cook wrote:
>
>
> On May 15, 2024 12:36:36 AM PDT, Peter Zijlstra wrote:
> >On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> >> For example, the most common case of overflow we've ever had has very
> >> much been array indexing. Now
On May 15, 2024 12:36:36 AM PDT, Peter Zijlstra wrote:
>On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
>> For example, the most common case of overflow we've ever had has very
>> much been array indexing. Now, sometimes that has actually been actual
>> undefined behavior, becau
On Wed, May 15, 2024 at 10:12:20AM -0700, Justin Stitt wrote:
> Hi Peter,
>
> On Wed, May 15, 2024 at 12:36 AM Peter Zijlstra wrote:
> >
> > On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> > > For example, the most common case of overflow we've ever had has very
> > > much been
Hi Peter,
On Wed, May 15, 2024 at 12:36 AM Peter Zijlstra wrote:
>
> On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> > For example, the most common case of overflow we've ever had has very
> > much been array indexing. Now, sometimes that has actually been actual
> > undefined b
On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> Now, another thing to do might be to treat assignments (and again,
> implicit casts) to 'size_t' specially. In most compilers (certainly in
> gcc), "size_t" is a special type.
>
> Now, in the kernel, we don't actually use __builtin
On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> For example, the most common case of overflow we've ever had has very
> much been array indexing. Now, sometimes that has actually been actual
> undefined behavior, because it's been overflow in signed variables,
> and those are "eas
Snipped all the bits where you are clearly correct.
On Mon, May 13, 2024 at 12:43:37PM -0700, Kees Cook wrote:
> > drivers/usb/class/usbtmc.c:852 usbtmc_generic_read() warn: potential
> > integer overflow from user 'max_transfer_size + 1'
> >842 * wMaxPacketSize – 1) to avoi
On Sat, May 11, 2024 at 07:19:49PM +0300, Dan Carpenter wrote:
> I'm pretty sure we've tried using a sanitizer before and it had too many
> false positives. So your solution is to annotate all the false
> positives?
That was the plan, yes. Based on current feedback, we'll be taking an
even more i
On Sun, May 12, 2024 at 09:09:08AM -0700, Linus Torvalds wrote:
> unsigned char *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
> ve
Am Sonntag, dem 12.05.2024 um 09:09 -0700 schrieb Linus Torvalds:
> On Sun, 12 May 2024 at 01:03, Martin Uecker 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 w
On Sun, 12 May 2024 at 01:03, Martin Uecker 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
Am Mittwoch, dem 08.05.2024 um 16:47 -0700 schrieb Linus Torvalds:
> On Wed, 8 May 2024 at 15:54, Kees Cook wrote:
> >
...
>
> No, the only point where that actually failed was then when a
> (non-overflowing, non-wrapping) final value was assigned to a 16-bit
> field, ie the problem only ever ha
I'm pretty sure we've tried using a sanitizer before and it had too many
false positives. So your solution is to annotate all the false
positives?
There are two main issue which make integer overflows complicated from
a static analysis perspective. 1) Places where it's intentional or
we don't c
...
> I think that would be a completely different area that might be worth
> looking at: instrumenting implicit casts for "drops bits". I'm afraid
> that it's just *so* common than we might not be able to do that
> sanely.
Things like:
buf[0] = val;
buf[1] = val >>= 8;
buf
...
> Going the other way is similar:
>
> all_bits = low_bits + ((u64) high_bits << 16) << 16);
>
> and again, the compiler will recognize this idiom and do the right
> thing (and if 'all_bits' is only 32-bit, the compiler will optimize
> the high bit noise away).
On a 32bit system the c
On Thu, May 09, 2024 at 12:15:40PM -0700, Linus Torvalds wrote:
> On Thu, 9 May 2024 at 11:48, Al Viro wrote:
> >
> > FWIW, the thing that somewhat worries me about having a helper along
> > the lines of combine_to_u64(low, high) is that
> > foo->splat = combine_to_u64(something, something
On Thu, 9 May 2024 at 11:48, Al Viro wrote:
>
> FWIW, the thing that somewhat worries me about having a helper along
> the lines of combine_to_u64(low, high) is that
> foo->splat = combine_to_u64(something, something_else);
> would be inviting hard-to-catch brainos -
> foo->splat =
On Thu, May 09, 2024 at 11:39:04AM -0700, Linus Torvalds wrote:
> .. it might also actually be a good idea *IF* we were to have some
> kind of "implicit cast drops bits" warning, in that the compiler for
> that case wouldn't remove the upper bits calculation, but would
> trigger a warning if they
On Thu, 9 May 2024 at 11:08, Linus Torvalds
wrote:
>
> Any half-way decent compiler will end up optimizing away the shifts
> and adds for the high bits because they see the assignment to
> 'all_bits'. There's no point in generating high bits that just get
> thrown away.
.. it might also actually
On Thu, 9 May 2024 at 10:54, Al Viro wrote:
>
> On Thu, May 09, 2024 at 08:38:28AM -0700, Linus Torvalds wrote:
>
> > Going the other way is similar:
> >
> > all_bits = low_bits + ((u64) high_bits << 16) << 16);
> >
> > and again, the compiler will recognize this idiom and do the right
> >
On Thu, May 09, 2024 at 08:38:28AM -0700, Linus Torvalds wrote:
> Going the other way is similar:
>
> all_bits = low_bits + ((u64) high_bits << 16) << 16);
>
> and again, the compiler will recognize this idiom and do the right
> thing (and if 'all_bits' is only 32-bit, the compiler will
On Thu, 9 May 2024 at 07:09, Theodore Ts'o wrote:
>
> struct ext2_super {
>...
>__u32time_lo;
>__u32time_high;
>...
> }
>
> time_t now;
>
> sb->time_low = now;
> sb->time_high = now >> 32;
>
> This is obviously (to a human) correct,
On Wed, May 08, 2024 at 11:11:35PM -0700, Kees Cook wrote:
> > I think it would be interesting in general to have some kind of
> > warning for "implicit cast drops bits".
> >
> > I fear that we'd have an enormous about of them, and maybe they'd be
> > unsolvable without making the code *much* ugli
On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> For example, the most common case of overflow we've ever had has very
> much been array indexing. Now, sometimes that has actually been actual
> undefined behavior, because it's been overflow in signed variables,
> and those are "eas
On Wed, 8 May 2024 at 16:47, Linus Torvalds
wrote:
>
> So *that* I feel could be something where you can warn without a ton
> of compiler smarts at all. If you see an *implicit* cast to unsigned
> and then the subsequent operations wraps around, it's probably worth
> being a lot more worried about
On Wed, 8 May 2024 at 16:47, Linus Torvalds
wrote:
>
> But again, maybe it would be worth looking into, at least for some
> limited cases. To go back to your particular example, it might be
> worth trying to limit it for unusual type sizes like implicit casts to
> 'u16' or bitfields: not because t
On Wed, 8 May 2024 at 15:54, Kees Cook wrote:
>
> Sure, but if the bar is omniscience, that's not a path forward.
I really don't think there's a lot of omniscience involved at all.
> I haven't really heard a viable counterproposal here.
So let me make the counter-proposal that you actually conc
On Wed, May 08, 2024 at 12:22:38PM +, David Laight wrote:
> Have you estimated the performance cost of checking the result of
> all integer arithmetic.
I hadn't included that in my already very long email as performance is
somewhat secondary to the correctness goals. Perhaps that was a mistake
On Wed, May 08, 2024 at 01:07:38PM -0700, Linus Torvalds wrote:
> On Wed, 8 May 2024 at 12:45, Kees Cook wrote:
> >
> > On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote:
> > > Example:
> > >
> > >static inline u32 __hash_32_generic(u32 val)
> > >{
> > > return val * G
On Wed, 8 May 2024 at 12:45, Kees Cook wrote:
>
> On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote:
> > Example:
> >
> >static inline u32 __hash_32_generic(u32 val)
> >{
> > return val * GOLDEN_RATIO_32;
> >}
>
> But what about:
>
> static inline u32 __item_offset
On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote:
> On Tue, 7 May 2024 at 16:27, Kees Cook wrote:
> [...]
> Put another way the absolute first and fundamental thing you should
> look at is to make sure tools don't complain about sane behavior.
Agreed, and I explicitly called this ou
On Tue, 7 May 2024 at 16:27, Kees Cook wrote:
>
> When I say "overflow", I mean "overflow and underflow", but more
> specifically I mean "wrap-around". This is not about "undefined
> behavior". We already demand from our compilers that all our arithmetic
> uses a well-defined overflow resolution s
From: Kees Cook
> Sent: 08 May 2024 00:28
>
> Over the last decade or so, our work hardening against weaknesses
> in various kernel APIs and eliminating the ambiguities in C language
> semantics have traditionally been somewhat off in one corner or another
> of the Linux codebase. This topic is go
43 matches
Mail list logo
