dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: dri-devel@lists.freedesktop.org,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	ojeda@kernel.org
Subject: Re: [PATCH 02/12] rust: num: reject Bounded::shr overshifts at build time
Date: Wed, 05 Aug 2026 05:55:42 +0000	[thread overview]
Message-ID: <20260805055542.7FD541F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260805-pramin-split-v1-2-ff3e84a75dac@nvidia.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The `Bounded` struct derives `Default`, which safely bypasses the required compile-time invariant checks in `Bounded::__new`.
--

--- Patch [2]: [PATCH 02/12] rust: num: reject Bounded::shr overshifts at build time ---
commit a9c888d83116c04cce01b982b58983e2d81e53c5
Author: Eliot Courtney <ecourtney@nvidia.com>

rust: num: reject Bounded::shr overshifts at build time

This commit updates shr in the Bounded struct to reject shifts that are
greater than or equal to the type's bit width at build time. This prevents
runtime panics or masked shift amounts.

> diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
> index dafe77782d797..f263107f001ed 100644
> --- a/rust/kernel/num/bounded.rs
> +++ b/rust/kernel/num/bounded.rs

[Severity: Medium]
This is a pre-existing issue, but does deriving Default on Bounded safely
bypass the required compile-time invariant checks?

Looking at the struct definition in rust/kernel/num/bounded.rs:

#[derive(Clone, Copy, Debug, Default, Hash)]
pub struct Bounded<T: Integer, const N: u32>(T);

The Bounded struct defines strict type invariants (such as N > 0 and
N <= T::BITS) which are enforced at compile time in its __new
constructor. However, deriving Default automatically implements a constructor
that initializes the tuple struct field via Self(T::default()), completely
bypassing __new.

Can safe downstream code construct an invalid instance using
Bounded::<u32, 0>::default()?

If this invalid instance is dereferenced, deref evaluates fits_within(0, 0),
calculating 32 - 0 = 32. Does left-shifting by 32 bits then trigger a runtime
panic in debug builds due to an overshift, or mask the shift amount in release
builds?

[ ... ]

> @@ -485,6 +485,7 @@ pub fn cast<U>(self) -> Bounded<U, N>
>      /// assert_eq!(v_shifted.get(), 0xff);
>      /// ```
>      pub fn shr<const SHIFT: u32, const RES: u32>(self) -> Bounded<T, RES> {
> +        const { assert!(SHIFT < T::BITS) }
>          const { assert!(RES + SHIFT >= N) }
>  
>          // SAFETY: We shift the value right by `SHIFT`, reducing the number of bits needed to

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-pramin-split-v1-0-ff3e84a75dac@nvidia.com?part=2

  reply	other threads:[~2026-08-05  5:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05  5:44 [PATCH 00/12] gpu: nova-core: add PRAMIN window support Eliot Courtney
2026-08-05  5:44 ` [PATCH 01/12] rust: io: add Region::try_subregion Eliot Courtney
2026-08-05 10:43   ` Gary Guo
2026-08-07  5:39     ` Eliot Courtney
2026-08-05  5:44 ` [PATCH 02/12] rust: num: reject Bounded::shr overshifts at build time Eliot Courtney
2026-08-05  5:55   ` sashiko-bot [this message]
2026-08-05  5:44 ` [PATCH 03/12] rust: num: add Bounded::shr_exact Eliot Courtney
2026-08-05  5:51   ` sashiko-bot
2026-08-05  5:44 ` [PATCH 04/12] gpu: nova-core: mm: Add VramAddress type Eliot Courtney
2026-08-05  5:49   ` sashiko-bot
2026-08-05  5:44 ` [PATCH 05/12] gpu: nova-core: mm: Implement Alignable and Debug for VramAddress Eliot Courtney
2026-08-05  5:44 ` [PATCH 06/12] gpu: nova-core: mm: Add PRAMIN window registers Eliot Courtney
2026-08-05  5:44 ` [PATCH 07/12] gpu: nova-core: mm: Add the memory management HAL Eliot Courtney
2026-08-05  5:44 ` [PATCH 08/12] gpu: nova-core: mm: Add support to use PRAMIN windows to write to VRAM Eliot Courtney
2026-08-05  5:44 ` [PATCH 09/12] docs: gpu: nova-core: Document the PRAMIN aperture mechanism Eliot Courtney
2026-08-05  5:44 ` [PATCH 10/12] gpu: nova-core: mm: Add GpuMm centralized memory manager Eliot Courtney
2026-08-05  5:52   ` sashiko-bot
2026-08-05  5:44 ` [PATCH 11/12] gpu: nova-core: Add self-test assertion macros and config option Eliot Courtney
2026-08-05  5:44 ` [PATCH 12/12] gpu: nova-core: mm: Add PRAMIN aperture self-tests Eliot Courtney

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=20260805055542.7FD541F000E9@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=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