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
Hi,
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 going to be much different as
it is ultimately abo
44 matches
Mail list logo