linux-kernel - Re: [PATCH v12 2/4] mm/slub: allow to set node and align in k[v]realloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <3AD3F7B5-679F-4DC8-968F-9FE991B56A5C@konsulko.se>
Date: Sat, 12 Jul 2025 14:43:51 +0200
From: Vitaly Wool <vitaly.wool@...sulko.se>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Harry Yoo <harry.yoo@...cle.com>,
 linux-mm@...ck.org,
 akpm@...ux-foundation.org,
 linux-kernel@...r.kernel.org,
 Uladzislau Rezki <urezki@...il.com>,
 Danilo Krummrich <dakr@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 rust-for-linux@...r.kernel.org,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>,
 Kent Overstreet <kent.overstreet@...ux.dev>,
 linux-bcachefs@...r.kernel.org,
 bpf@...r.kernel.org,
 Herbert Xu <herbert@...dor.apana.org.au>,
 Jann Horn <jannh@...gle.com>,
 Pedro Falcato <pfalcato@...e.de>
Subject: Re: [PATCH v12 2/4] mm/slub: allow to set node and align in
 k[v]realloc



> On Jul 11, 2025, at 5:43 PM, Vlastimil Babka <vbabka@...e.cz> wrote:
> 
> On 7/11/25 10:58, Harry Yoo wrote:
>> On Wed, Jul 09, 2025 at 07:24:41PM +0200, Vitaly Wool wrote:
>>> Reimplement k[v]realloc_node() to be able to set node and
>>> alignment should a user need to do so. In order to do that while
>>> retaining the maximal backward compatibility, add
>>> k[v]realloc_node_align() functions and redefine the rest of API
>>> using these new ones.
>>> 
>>> While doing that, we also keep the number of  _noprof variants to a
>>> minimum, which implies some changes to the existing users of older
>>> _noprof functions, that basically being bcachefs.
>>> 
>>> With that change we also provide the ability for the Rust part of
>>> the kernel to set node and alignment in its K[v]xxx
>>> [re]allocations.
>>> 
>>> Signed-off-by: Vitaly Wool <vitaly.wool@...sulko.se>
>>> ---
>>> fs/bcachefs/darray.c   |  2 +-
>>> fs/bcachefs/util.h     |  2 +-
>>> include/linux/bpfptr.h |  2 +-
>>> include/linux/slab.h   | 38 +++++++++++++++----------
>>> lib/rhashtable.c       |  4 +--
>>> mm/slub.c              | 64 +++++++++++++++++++++++++++++-------------
>>> 6 files changed, 72 insertions(+), 40 deletions(-)
>> 
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index c4b64821e680..6fad4cdea6c4 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -4845,7 +4845,7 @@ void kfree(const void *object)
>>> EXPORT_SYMBOL(kfree);
>>> 
>>> static __always_inline __realloc_size(2) void *
>>> -__do_krealloc(const void *p, size_t new_size, gfp_t flags)
>>> +__do_krealloc(const void *p, size_t new_size, unsigned long align, gfp_t flags, int nid)
>>> {
>>> void *ret;
>>> size_t ks = 0;
>>> @@ -4859,6 +4859,20 @@ __do_krealloc(const void *p, size_t new_size, gfp_t flags)
>>> if (!kasan_check_byte(p))
>>> return NULL;
>>> 
>>> + /* refuse to proceed if alignment is bigger than what kmalloc() provides */
>>> + if (!IS_ALIGNED((unsigned long)p, align) || new_size < align)
>>> + return NULL;
>> 
>> Hmm but what happens if `p` is aligned to `align`, but the new object is not?
>> 
>> For example, what will happen if we  allocate object with size=64, align=64
>> and then do krealloc with size=96, align=64...
>> 
>> Or am I missing something?
> 
> Good point. We extended the alignment guarantees in commit ad59baa31695
> ("slab, rust: extend kmalloc() alignment guarantees to remove Rust padding")
> for rust in a way that size 96 gives you alignment of 32. It assumes that
> rust side will ask for alignments that are power-of-two and sizes that are
> multiples of alignment. I think if that assumption is still honored than
> this will keep working, but the check added above (is it just a sanity check
> or something the rust side relies on?) doesn't seem correct?
> 

It is a sanity check and it should have looked like this:

        if (!IS_ALIGNED((unsigned long)p, align) && new_size <= ks)
                return NULL;

and the reasoning for this is the following: if we don’t intend to reallocate (new size is not bigger than the original size), but the user requests a larger alignment, it’s a miss. Does that sound reasonable?

~Vitaly