Message-ID: <20251125115419.304dd2a9@pumpkin>
Date: Tue, 25 Nov 2025 11:54:19 +0000
From: david laight <david.laight@...box.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, Kees Cook
 <kees@...nel.org>, Vlastimil Babka <vbabka@...e.cz>, Christoph Lameter
 <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>, David Rientjes
 <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, Andrew Morton
 <akpm@...ux-foundation.org>, Roman Gushchin <roman.gushchin@...ux.dev>,
 Hyeonggon Yoo <42.hyeyoo@...il.com>, "Gustavo A . R . Silva"
 <gustavoars@...nel.org>, Bill Wendling <morbo@...gle.com>, Justin Stitt
 <justinstitt@...gle.com>, Jann Horn <jannh@...gle.com>, Przemek Kitszel
 <przemyslaw.kitszel@...el.com>, Marco Elver <elver@...gle.com>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Sasha Levin
 <sashal@...nel.org>, linux-mm@...ck.org, Randy Dunlap
 <rdunlap@...radead.org>, Miguel Ojeda <ojeda@...nel.org>, Vegard Nossum
 <vegard.nossum@...cle.com>, Harry Yoo <harry.yoo@...cle.com>, Nathan
 Chancellor <nathan@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Nick
 Desaulniers <nick.desaulniers+lkml@...il.com>, Jonathan Corbet
 <corbet@....net>, Jakub Kicinski <kuba@...nel.org>, Yafang Shao
 <laoar.shao@...il.com>, Tony Ambardar <tony.ambardar@...il.com>, Alexander
 Lobakin <aleksander.lobakin@...el.com>, Jan Hendrik Farr <kernel@...rr.cc>,
 Alexander Potapenko <glider@...gle.com>, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org, linux-doc@...r.kernel.org,
 llvm@...ts.linux.dev
Subject: Re: [PATCH v5 2/4] slab: Introduce kmalloc_obj() and family

On Tue, 25 Nov 2025 01:09:42 +0000
Matthew Wilcox <willy@...radead.org> wrote:

> On Mon, Nov 24, 2025 at 03:30:19PM -0800, Linus Torvalds wrote:
> > That all a very standard thing in assembly programming, which this is
> > all about. 'entry' is a signed offset from its own address.  
> 
> I used to be an assembly programmer ... 28 years ago.  I've mostly put
> that world out of my mind (and being able to write a 20,000 instruction
> ARM32 program entirely in assembly is just not that useful an
> accomplishment to put on my CV).  Anyway, this isn't the point ...
> 
> > > The warning is ... not the best phrased, but in terms of divining the
> > > programmer's intent, I genuinely don't know if this code is supposed
> > > to zero-extend or sign-extend the s32 to unsigned long.  
> > 
> > What?
> > 
> > A signed value gets sign-extended when cast to a larger type. That's
> > how all of this always works. Casting a signed value to 'unsigned
> > long' will set the high bits in the result.
> > 
> > That's pretty much the *definition* of a signed value. It gets
> > sign-extended when used, and then obviously it becomes a large
> > unsigned value, but this is how two's complement addition
> > fundamentally works.  
> 
> Yes, agreed.
> 
> > So honestly, what's the problem with this code?
> > 
> > The warning makes no sense, and is garbage. Are we not allowed to add
> > signed integers to unsigned 64-bit values now, because that addition
> > involves that cast of a signed 32-bit entry to an unsigned 64-bit one?
> > 
> > There is NO WAY that warning is valid, it's; not *ever* something we
> > should enable, and the fact that you people are discussing it as such
> > is just crazy.
> > 
> > That code would not be improved at all by adding another cast (to
> > first cast that s32 to 'long', in order to then add it to 'unsigned
> > long').
> > 
> > Imagine how many other places you add integers to 'unsigned long'.
> > EVERY SINGLE ONE of those places involves sign-extending the integer
> > and then doing arithmetic in unsigned.  
> 
> I have bad news.  Rust requires it.
> 
> fn add(base: u64, off: i32) -> u64 {
>     base + off
> }
> 
> error[E0308]: mismatched types
>  --> add.rs:2:12  
>   |
> 2 |     base + off
>   |            ^^^ expected `u64`, found `i32`
> 
> error[E0277]: cannot add `i32` to `u64`
>  --> add.rs:2:10  
>   |
> 2 |     base + off
>   |          ^ no implementation for `u64 + i32`
>   |
>   = help: the trait `Add<i32>` is not implemented for `u64`
>   = help: the following other types implement trait `Add<Rhs>`:
>             <u64 as Add>
>             <u64 as Add<&u64>>
>             <&'a u64 as Add<u64>>
>             <&u64 as Add<&u64>>
> 
> so the Rust language people have clearly decided that this is too
> complicated for your average programmer to figure out, and you need
> explicit casts to make it work.
> 

Jeepers...
As I've found looking at min_t() you can't trust kernel programmers
(never mind 'average' ones) to use the correct cast.
It wouldn't surprise be if the casts cause more bugs that the automatic
conversions that C does.

It wouldn't be as bad if there were separate 'casts' for widening and narrowing.
You also need the compiler to be doing 'value tracking' rather than just
looking at the types.
If I do:
	int len = read(.....);
	if (len < 0)
		return -1;
	if (len > sizeof (...))
		...
then -Wsign-compare complains, but a statically_true(len >= 0) is fine.

	David

Powered by blists - more mailing lists