On 5/12/26 14:54, Marco Elver wrote: > On Tue, 12 May 2026 at 12:37, 'Vlastimil Babka (SUSE)' via kasan-dev > <[email protected]> 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.
