Message-ID: <CAHk-=wgn-2ieKtaHAXLLge1UFckLKa88vo3XOdLz+fP+DLpHGA@mail.gmail.com>
Date: Mon, 24 Nov 2025 15:30:19 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Matthew Wilcox <willy@...radead.org>
Cc: 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 Mon, 24 Nov 2025 at 13:44, Matthew Wilcox <willy@...radead.org> wrote:
>
> This third one is interesting.

Why?

> include/linux/jump_label.h:126:44: error: conversion to ‘long unsigned int’ from ‘s32’ {aka ‘int’} may change the sign of the result [-Werror=sign-conversion]
>   126 |         return (unsigned long)&entry->code + entry->code;
>
> static inline unsigned long jump_entry_code(const struct jump_entry *entry)
> {
>         return (unsigned long)&entry->code + entry->code;
> }

I'm not seeing the confusion. 'entry->code' is a signed 32-bit entry,
and we're adding it to the address of the entry itself (well, the
address of the 'code' member of the entry, but it's the first thing in
that struct, so same thing in this case).

The next function in that file (which uses "target", which is *not*
the first entry in the struct) then makes it clear that we actually do
all these relative offsets relative to the address of the relative
offset itself, not the base address of the struct.

But that's actually just a random implementation decision.

That all a very standard thing in assembly programming, which this is
all about. 'entry' is a signed offset from its own address.

And yes, the exact thing you are relative to may differ. When looking
at instruction sets, sometimes the relative address is relative to the
beginning of the current instruction (I think m68k did that), and
sometimes it's relative to the *end* of the instruction (I think x86
does that).  It could even be relative to the actual byte *inside* the
instruction, although I can't think of an example of that.

Sometimes it's relative with a shift (basically all fixed-size
instruction CPU's do that since instructions are all mutually
aligned).

So that's whole "exactly what is it relative to, what are the sizes
involved, and are there maybe shifts" etc just a design choice.

The whole "offset is relative to its own address" that this code uses
is probably the simplest form it can have.

> 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.

This is bog-standard and happens all over the place. We have things like

        unsigned long xyz = -1;

in various places, this is not some kind of unclear area of the standard.

And for all the same reasons, the programmers intent is obvious too.
'code' is s32 and is signed for a reason - so that you have a +-2GB
relative offset. And this is not some kind of unusual pattern when
we're talking about relative branch targets.

Now, if you don't know about things like relative branch targets in
low-level assembly, maybe this is code you have to look over several
times, but this code is literally ABOUT re-writing branches in
assembly language, and this kind of pattern where you use relative
offsets is traditional.

And yes, the offsets are literally smaller than the address space, and
the signed offsets get sign-extended as part of this all: that's very
traditional too.

Basically no 64-bit CPU has 64-bit branch offsets (and few 32-bit
CPU's have full 32-bit branch offsets - x86 happens to do it, but most
RISC CPU's tend to have branch offsets that are in the ~20 bit range
and if you have big relative branches you need to do those with a
separate constant section or something like that).

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.

                 Linus

Powered by blists - more mailing lists