dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: ojeda@kernel.org, "Alexandre Courbot" <acourbot@nvidia.com>,
	gary@garyguo.net, dri-devel@lists.freedesktop.org,
	"Danilo Krummrich" <dakr@kernel.org>
Subject: Re: [PATCH v9 8/9] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN
Date: Wed, 30 Sep 2026 02:52:54 +0000	[thread overview]
Message-ID: <20260930025254.AEB5B1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260930-chid-v9-8-0d6cca376cff@nvidia.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rust: id_pool: panic on find_unused_id with 0-capacity pool
--

commit 424a508011ab0d65530fe9badebd792ee11ae3f5
Author: Eliot Courtney <ecourtney@nvidia.com>

rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN

This patch modifies IdPool::with_capacity to no longer round up the initial
capacity to BitmapVec::MAX_INLINE_LEN, allowing for a capacity of 0. It also
adjusts grow_request to ensure that a 0-capacity pool grows to at least
BitmapVec::MAX_INLINE_LEN instead of remaining 0.

> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index 06a4c71c4c6c9..4f329249df9d0 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -112,13 +112,8 @@ pub fn new() -> Self {
>      }
>  
>      /// Constructs a new [`IdPool`] with space for a specific number of bits.
> -    ///
> -    /// A capacity below [`MAX_INLINE_LEN`] is adjusted to [`MAX_INLINE_LEN`].
> -    ///
> -    /// [`MAX_INLINE_LEN`]: BitmapVec::MAX_INLINE_LEN
>      #[inline]
>      pub fn with_capacity(num_ids: usize, flags: Flags) -> Result<Self, AllocError> {
> -        let num_ids = usize::max(num_ids, BitmapVec::MAX_INLINE_LEN);
>          let map = BitmapVec::new(num_ids, flags)?;
>          Ok(Self { map })
>      }

[Severity: High]
Can allowing a capacity of 0 here introduce a regression where finding unused
IDs triggers a kernel panic?

If a user initializes a 0-capacity pool and calls find_unused_id() or
reserve_ids(), the offset is passed directly to the underlying Bitmap
functions.

For example, in IdPool::find_unused_id():

rust/kernel/id_pool.rs:IdPool::find_unused_id() {
    ...
    Some(UnusedId {
        id: self.map.next_zero_bit(offset)?,
        pool: self,
    })
    ...
}

The underlying Bitmap::next_zero_bit() function enforces a bounds check
via bitmap_assert!(start < self.len()).

For a 0-capacity pool with start=0, this evaluates to 0 < 0, triggering an
unconditional panic when the CONFIG_RUST_BITMAP_HARDENED configuration is
enabled.

Should we handle 0-capacity gracefully and return None to signal the need for
a grow_request() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-chid-v9-0-0d6cca376cff@nvidia.com?part=8

  reply	other threads:[~2026-09-30  2:52 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30  2:42 [PATCH v9 0/9] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-09-30  2:42 ` [PATCH v9 1/9] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-09-30  2:42 ` [PATCH v9 2/9] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-09-30  2:42 ` [PATCH v9 3/9] rust: sizes: add sub-1K size constants Eliot Courtney
2026-09-30  9:12   ` Miguel Ojeda
2026-09-30 11:04     ` Alexandre Courbot
2026-09-30 11:22       ` Miguel Ojeda
2026-10-01  0:40         ` Gary Guo
2026-10-01  5:23           ` Eliot Courtney
2026-09-30  2:42 ` [PATCH v9 4/9] rust: sizes: implement SizeConstants for Alignment Eliot Courtney
2026-09-30  9:12   ` Miguel Ojeda
2026-09-30  2:42 ` [PATCH v9 5/9] rust: use Alignment size constants Eliot Courtney
2026-09-30  2:42 ` [PATCH v9 6/9] rust: bitmap: add contiguous area operations Eliot Courtney
2026-09-30  4:41   ` Yury Norov
2026-09-30  2:42 ` [PATCH v9 7/9] rust: id_pool: add contiguous ID reservation Eliot Courtney
2026-09-30  2:51   ` sashiko-bot
2026-09-30  4:50   ` Yury Norov
2026-09-30  2:42 ` [PATCH v9 8/9] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN Eliot Courtney
2026-09-30  2:52   ` sashiko-bot [this message]
2026-09-30  5:05   ` Yury Norov
2026-10-01  6:20     ` Alexandre Courbot
2026-10-08 15:47       ` Yury Norov
2026-10-08 16:04         ` Gary Guo
2026-10-09 14:07         ` Alexandre Courbot
2026-09-30  2:42 ` [PATCH v9 9/9] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-10-08 15:04 ` [PATCH v9 0/9] rust: Add support for reserving of ranges of IDs Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260930025254.AEB5B1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=ojeda@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox