linux-kernel - Re: [PATCH v6 0/4] rust: adds Bitmap API, ID pool and bindings

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: <Z-rFPhrNTB8t5xTF@thinkpad>
Date: Mon, 31 Mar 2025 12:39:43 -0400
From: Yury Norov <yury.norov@...il.com>
To: Burak Emir <bqe@...gle.com>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	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 <benno.lossin@...ton.me>,
	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
Subject: Re: [PATCH v6 0/4] rust: adds Bitmap API, ID pool and bindings

On Thu, Mar 27, 2025 at 04:16:10PM +0000, Burak Emir wrote:
> This series adds a Rust bitmap API for porting the approach from
> commit 15d9da3f818c ("binder: use bitmap for faster descriptor lookup")
> to Rust. The functionality in dbitmap.h makes use of bitmap and bitops.
> 
> The Rust bitmap API provides a safe abstraction to underlying bitmap
> and bitops operations. For now, only includes method necessary for
> dbitmap.h, more can be added later. We perform bounds checks for
> hardening, violations are programmer errors that result in panics.

So, I hung around and found the bit_set crate. Rust implements BitSet,
BitVec, and it seems it has functionality that you bind here.

I didn't find any discussions related to the bit_set in kernel context.
Is it possible to use it in kernel? If not, can you mention that in commit
message? If yes, I think you should consider to use internal language
tools.

> We include set_bit_atomic and clear_bit_atomic operations. One has
> to avoid races with non-atomic operations, which is ensure by the
> Rust type system: either callers have shared references &bitmap in
> which case the mutations are atomic operations. Or there is a
> exclusive reference &mut bitmap, in which case there is no concurrent
> access.
> 
> This version includes an optimization to represent the bitmap inline,
> as suggested by Yury.
> 
> We introduce a Rust API that would replace (dbitmap.h) in file id_pool.rs. 
> This data structure is tightly coupled to the bitmap API. Includes an example of usage
> that requires releasing a spinlock, as expected in Binder driver.
> 
> This is v6 of a patch introducing Rust bitmap API [v5]. Thanks
> for all the helpful comments, this series has improved significantly
> as a result of your work.
> 
> Not adding separate unit tests: the Rust unit test infrastructure
> is very new, and there does not seem to be benchmarking support
> for Rust tests yet.

I don't understand this.

Benchmarking is a very simple procedure - as simple as surrounding
blocks of tested code with ktime_get(). And I see that Alice even
implemented the Ktime class last year.

> Are the # Examples tests enough?
> Alternatively, can we add more test cases to those until 
> the unit test infrastructure is in place?

I encourage you to implement the tests as normal kernel tests - in
source files that may be enabled in config. I can't insist on that,
and will not block the series because of lack of benchmarks and
tests written in a traditional way.

But to me, scattered wrongly formatted commented-out in-place way of
writing tests is something fundamentally wrong. Not mentioning that
it bloats source files, making them harder to read.

Thanks,
Yury