RE: [RFC] Mitigating unexpected arithmetic overflow

2024-05-18 Thread David Laight
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 +

RE: [RFC] Mitigating unexpected arithmetic overflow

2024-05-18 Thread David Laight
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-17 Thread Matthew Wilcox
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-17 Thread Theodore Ts'o
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-17 Thread Fangrui Song
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-17 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-17 Thread Jonas Oberhauser
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Theodore Ts'o
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Justin Stitt
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Peter Zijlstra
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-16 Thread Peter Zijlstra
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-15 Thread Justin Stitt
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-15 Thread Peter Zijlstra
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-15 Thread Peter Zijlstra
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-14 Thread Dan Carpenter
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-13 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-13 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-12 Thread Martin Uecker
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-12 Thread 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 way too many false positives. I'm pretty sure there will be a ton of "intentional

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-12 Thread Martin Uecker
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-11 Thread Dan Carpenter
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

RE: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread David Laight
... > 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

RE: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread David Laight
... > 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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Al Viro
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Linus Torvalds
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 =

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Al Viro
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Linus Torvalds
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Linus Torvalds
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 > >

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Al Viro
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Linus Torvalds
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,

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-09 Thread Theodore Ts'o
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Linus Torvalds
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Linus Torvalds
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Linus Torvalds
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Linus Torvalds
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Kees Cook
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

Re: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread Linus Torvalds
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

RE: [RFC] Mitigating unexpected arithmetic overflow

2024-05-08 Thread David Laight
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

[RFC] Mitigating unexpected arithmetic overflow

2024-05-07 Thread Kees Cook
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