linux-kernel - Re: [PATCH] rust: slab: add basic slab module

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: <57b01f59-f4b1-4f5f-bb63-fd8eea19b7ba@suse.cz>
Date: Fri, 26 Sep 2025 19:11:59 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Elijah Wright <git@...jahs.space>, Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Uladzislau Rezki <urezki@...il.com>, linux-mm@...ck.org
Subject: Re: [PATCH] rust: slab: add basic slab module

On 9/26/25 18:58, Danilo Krummrich wrote:
> On Fri Sep 26, 2025 at 6:32 PM CEST, Vlastimil Babka wrote:
>> On 9/26/25 17:55, Danilo Krummrich wrote:
> 
>>> 	// Frees foo and causes the "zombie" cache to actually be destroyed.
>>> 	kmem_cache_free(foo);
>>
>> The free will be fine. But not cause the cache destruction, as that would
>> require checks on each free. But should be fine wrt safety if we only leak
>> some memory due to a wrong usage, no?
> 
> Yes, technically that's safe, but we wouldn't prevent the leak, which still
> is not desirable (and not our ambition for a Rust API).
> 
> From a C standpoint, both the warning and the cache leak could be solved by
> making kmem_cache_destroy() fallible as you mentioned previously.
> 
> On the Rust side the cache would be represented with a struct KmemCache<T>
> (where T is the type that should be allocated by the cache).
> 
> kmem_cache_destroy() would be called from KmemCache<T>::drop(), which is not
> fallible. But even if it were, we can't enforce that users keep the KmemCache
> instance alive as long as there are allocations.
> 
> So, either we always keep the KmemCache<T> alive for the whole module lifetime
> (which depending on whether its built-in or not could be considered a memory
> leak as well). Or we ensure that the last kmem_cache_free() also frees the cache
> if kmem_cache_destroy() was called previously.

The rust wrapper side could do that so we don't have to add that check in
all kmem_cache_free() calls, maybe?
Also either way it could perhaps be difficult/expensive (memory barriers
etc) to properly handle a racing kmem_cache_free() and kmem_cache_destroy()
in a way that ensures the cache is always being destroyed, and not have the
kmem_cache_destroy() observe the destroy was premature, while the racing
kmem_cache_free() doesn't yet observe that destroy was attempted, and not
try to remove it.
> OOC, does the cache pointer remain valid if kmem_cache_destroy() is called while
> allocations still exist? I.e. is this, except for the WARN(), valid code?
> 
> 	kmem_cache_destroy(cache);
> 	kmem_cache_free(foo);
> 	kmem_cache_destroy(cache);
Currently not, but making it valid hopefully should not be difficult,
without affecting fast path.

> At a quick glance it appears to me that things would go south in
> kmem_cache_release(). Anyways, I don't think it would help, if it would be the
> case.

It would help if it was safe and the rust wrapper side did this retry?