Re: [PATCH v4 2/3] slab: improve KMALLOC_PARTITION_RANDOM randomness

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 5/12/26 14:54, Marco Elver wrote:
> On Tue, 12 May 2026 at 12:37, 'Vlastimil Babka (SUSE)' via kasan-dev
> <kasan-dev@googlegroups.com> wrote:
>> On 5/11/26 22:00, Marco Elver wrote:
>> > When using CONFIG_KMALLOC_PARTITION_RANDOM, _RET_IP_ was previously used
>> > to identify the allocation site. _RET_IP_, however, evaluates to the
>> > caller's parent's instruction pointer rather than the actual allocation
>> > site; this would lead to collisions where a function performs multiple
>> > allocations.
>> >
>> > With the generalization to kmalloc_token_t, we now generate the token at
>> > the outermost macro, and using _THIS_IP_ would fix this for all cases.
>>
>> Hm but it means in patch 1 we make things even worse and then fix them
>> again, and also improve what was suboptimal prior to the series.
>> Would it be instead possible to reorder patches 1 and 2 so we improve the
>> current state first, and then introduce typed partitioning without any
>> changes to the randomized one? (aside from changing the previously correcly
>> used cases _RET_IP_ to _CODE_LOCATION_).
> 
> It won't work (it could be made to work if _THIS_IP_ wasn't broken).
> The compiler is supposed to maintain semantics of a static variable in
> a function, even inline functions, and refer to the same static
> variable -- and because kmalloc_type is an inline function, if
> _CODE_LOCATION_ is the non-_THIS_IP_ version, it'd break.

Oh, I see.

> Even if _THIS_IP_ wasn't broken, the other complication is introducing
> the slab.c vs. outside use of kmalloc_type differentiation.
> 
> Both these problems go away if we make this patch 2 (using
> _CODE_LOCATION_ on the outer macro, not in an inline function).
> 
> While I understand that maybe we could have considered this as a
> stable backport, I think it's borderline; the feature isn't broken
> per-se, just slightly lower randomness than perhaps intended if size
> is a constant expression. A minimal fix prior to the macro rework
> currently eludes me.

Fair enough. I wanted to avoid a bisection hazard, but since it's not
feasible and it shouldn't actually break anything (compilation, run), it's
acceptable. Thanks.