| 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
| ||
|
Message-Id: <57A5C181-9D72-483C-BC7D-DCE744C508D1@collabora.com>
Date: Tue, 25 Nov 2025 16:55:32 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Filipe Xavier <felipeaggger@...il.com>
Cc: 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>,
Danilo Krummrich <dakr@...nel.org>,
rust-for-linux@...r.kernel.org,
felipe_life@...e.com,
linux-kernel@...r.kernel.org,
Lyude Paul <lyude@...hat.com>
Subject: Re: [PATCH v5] rust: add new macro for common bitmap operations
Hi Filipe,
Thanks for working on this, this is much cleaner and will cut down on the
boilerplate. It works just fine on Tyr.
> On 9 Nov 2025, at 10:31, Filipe Xavier <felipeaggger@...il.com> wrote:
>
> We have seen a proliferation of mod_whatever::foo::Flags
> being defined with essentially the same implementation
> for BitAnd, BitOr, contains and etc.
>
> This macro aims to bring a solution for this,
> allowing to generate these methods for user-defined structs.
> With some use cases in KMS and upcoming GPU drivers.
>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
> Signed-off-by: Filipe Xavier <felipeaggger@...il.com>
> Suggested-by: Daniel Almeida <daniel.almeida@...labora.com>
> Suggested-by: Lyude Paul <lyude@...hat.com>
> ---
> Changes in v5:
> - Docs: improve macro documentation.
> - Link to v4: https://lore.kernel.org/r/20251026-feat-add-bitmask-macro-v4-1-e1b59b4762bc@gmail.com
>
> Changes in v4:
> - Use enum: changed flag type from struct to enum.
> - Minor fix: airect casting (flag as $ty) instead of field access (.0).
> - Link to v3: https://lore.kernel.org/r/20250411-feat-add-bitmask-macro-v3-1-187bd3e4a03e@gmail.com
>
> Changes in v3:
> - New Feat: added support to declare flags inside macro use.
> - Minor fixes: used absolute paths to refer to items, fix rustdoc and fix example cases.
> - Link to v2: https://lore.kernel.org/r/20250325-feat-add-bitmask-macro-v2-1-d3beabdad90f@gmail.com
>
> Changes in v2:
> - rename: change macro and file name to impl_flags.
> - negation sign: change char for negation to `!`.
> - transpose docs: add support to transpose user provided docs.
> - visibility: add support to use user defined visibility.
> - operations: add new operations for flag,
> to support use between bit and bitmap, eg: flag & flags.
> - code style: small fixes to remove warnings.
> - Link to v1: https://lore.kernel.org/r/20250304-feat-add-bitmask-macro-v1-1-1c2d2bcb476b@gmail.com
> ---
> rust/kernel/impl_flags.rs | 229 ++++++++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 2 +
> rust/kernel/prelude.rs | 1 +
> 3 files changed, 232 insertions(+)
>
> diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..c1086677678408f46c4832e2110c761fab017055
> --- /dev/null
> +++ b/rust/kernel/impl_flags.rs
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/// Common helper for declaring bitflag and bitmask types.
> +///
> +/// This macro handles:
> +/// - A struct representing a bitmask, and an enumerator representing bitflags which
> +/// may be used in the aforementioned bitmask.
> +/// - Implementations of common bitmap op. ([`::core::ops::BitOr`], [`::core::ops::BitAnd`], etc.).
> +/// - Utility methods such as `.contains()` to check flags.
> +///
> +/// # Examples
> +///
> +/// Defining and using impl_flags:
> +///
> +/// ```
> +/// use kernel::impl_flags;
> +///
> +/// impl_flags!(
> +/// /// Represents multiple permissions.
> +/// #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
> +/// pub struct Permissions(u32);
> +/// /// Represents a single permission.
> +/// #[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +/// pub enum Permission {
> +/// Read = 1 << 0,
> +/// Write = 1 << 1,
> +/// Execute = 1 << 2,
> +/// }
> +/// );
> +///
> +/// // Combine multiple permissions using operation OR (`|`).
> +/// let read_write: Permissions = Permission::Read | Permission::Write;
> +///
> +/// assert!(read_write.contains(Permission::Read));
> +/// assert!(read_write.contains(Permission::Write));
> +/// assert!(!read_write.contains(Permission::Execute));
> +///
> +/// // Removing a permission with operation AND (`&`).
> +/// let read_only: Permissions = read_write & Permission::Read;
> +/// assert!(read_only.contains(Permission::Read));
> +/// assert!(!read_only.contains(Permission::Write));
> +///
> +/// // Toggling permissions with XOR (`^`).
> +/// let toggled: Permissions = read_only ^ Permission::Read;
> +/// assert!(!toggled.contains(Permission::Read));
> +///
> +/// // Inverting permissions with negation (`!`).
> +/// let negated = !read_only;
> +/// assert!(negated.contains(Permission::Write));
> +/// ```
> +#[macro_export]
> +macro_rules! impl_flags {
> + (
> + $(#[$outer_flags:meta])*
> + $vis_flags:vis struct $flags:ident($ty:ty);
> +
> + $(#[$outer_flag:meta])*
> + $vis_flag:vis enum $flag:ident {
> + $(
> + $(#[$inner_flag:meta])*
> + $name:ident = $value:expr
> + ),+ $( , )?
> + }
> + ) => {
> + $(#[$outer_flags])*
> + #[repr(transparent)]
> + $vis_flags struct $flags($ty);
> +
> + $(#[$outer_flag])*
> + #[repr($ty)]
> + $vis_flag enum $flag {
> + $(
> + $(#[$inner_flag])*
> + $name = $value
> + ),+
> + }
> +
> + impl ::core::convert::From<$flag> for $flags {
> + #[inline]
> + fn from(value: $flag) -> Self {
> + Self(value as $ty)
> + }
> + }
> +
> + impl ::core::convert::From<$flags> for $ty {
> + #[inline]
> + fn from(value: $flags) -> Self {
> + value.0
> + }
> + }
> +
> + impl ::core::ops::BitOr for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitor(self, rhs: Self) -> Self::Output {
> + Self(self.0 | rhs.0)
> + }
> + }
> +
> + impl ::core::ops::BitOrAssign for $flags {
> + #[inline]
> + fn bitor_assign(&mut self, rhs: Self) {
> + *self = *self | rhs;
> + }
> + }
> +
> + impl ::core::ops::BitAnd for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitand(self, rhs: Self) -> Self::Output {
> + Self(self.0 & rhs.0)
> + }
> + }
> +
> + impl ::core::ops::BitAndAssign for $flags {
> + #[inline]
> + fn bitand_assign(&mut self, rhs: Self) {
> + *self = *self & rhs;
> + }
> + }
> +
> + impl ::core::ops::BitOr<$flag> for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitor(self, rhs: $flag) -> Self::Output {
> + self | Self::from(rhs)
> + }
> + }
> +
> + impl ::core::ops::BitOrAssign<$flag> for $flags {
> + #[inline]
> + fn bitor_assign(&mut self, rhs: $flag) {
> + *self = *self | rhs;
> + }
> + }
> +
> + impl ::core::ops::BitAnd<$flag> for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitand(self, rhs: $flag) -> Self::Output {
> + self & Self::from(rhs)
> + }
> + }
> +
> + impl ::core::ops::BitAndAssign<$flag> for $flags {
> + #[inline]
> + fn bitand_assign(&mut self, rhs: $flag) {
> + *self = *self & rhs;
> + }
> + }
> +
> + impl ::core::ops::BitXor for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitxor(self, rhs: Self) -> Self::Output {
> + Self(self.0 ^ rhs.0)
> + }
> + }
> +
> + impl ::core::ops::BitXorAssign for $flags {
> + #[inline]
> + fn bitxor_assign(&mut self, rhs: Self) {
> + *self = *self ^ rhs;
> + }
> + }
> +
> + impl ::core::ops::Not for $flags {
> + type Output = Self;
> + #[inline]
> + fn not(self) -> Self::Output {
> + Self(!self.0)
> + }
> + }
> +
> + impl ::core::ops::BitOr for $flag {
> + type Output = $flags;
> + #[inline]
> + fn bitor(self, rhs: Self) -> Self::Output {
> + $flags(self as $ty | rhs as $ty)
> + }
> + }
> +
> + impl ::core::ops::BitAnd for $flag {
> + type Output = $flags;
> + #[inline]
> + fn bitand(self, rhs: Self) -> Self::Output {
> + $flags(self as $ty & rhs as $ty)
> + }
> + }
> +
> + impl ::core::ops::BitXor for $flag {
> + type Output = $flags;
> + #[inline]
> + fn bitxor(self, rhs: Self) -> Self::Output {
> + $flags(self as $ty ^ rhs as $ty)
> + }
> + }
> +
> + impl ::core::ops::Not for $flag {
> + type Output = $flags;
> + #[inline]
> + fn not(self) -> Self::Output {
> + $flags(!(self as $ty))
> + }
> + }
> +
> + impl ::core::ops::BitXor<$flag> for $flags {
> + type Output = Self;
> + #[inline]
> + fn bitxor(self, rhs: $flag) -> Self::Output {
> + self ^ Self::from(rhs)
> + }
> + }
> +
> + impl $flags {
> + /// Returns an empty instance of `type` where no flags are set.
> + #[inline]
> + pub const fn empty() -> Self {
> + Self(0)
> + }
> +
> + /// Checks if a specific flag is set.
> + #[inline]
> + pub fn contains(self, flag: $flag) -> bool {
> + (self.0 & flag as $ty) == flag as $ty
> + }
Please keep the current implementation but also provide a contains_any(flags: Flags)
and contains_all(flags: Flags) method so users don't have to manually do:
if flags.contains(flag1) || flags.contains(flag2) ... flags.contains(flagN), or:
if flags.contains(flag1) && flags.contains(flags2) .. etc
This is much faster:
if flags.contains_any(flag1 | flag2 | flagN), or:
if flags.contains_all(flag1 | flag2 | flagN)
With the small change above,
Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
> + }
> + };
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 28007be98fbad0e875d7e5345e164e2af2c5da32..57b789315deda3d2fa8961af79cad4462fff7fa8 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -67,6 +67,8 @@
> #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
> pub mod firmware;
> pub mod fs;
> +#[doc(hidden)]
> +pub mod impl_flags;
> pub mod init;
> pub mod io;
> pub mod ioctl;
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index baa774a351ceeb995a2a647f78a27b408d9f3834..085f23502544c80c2202eacd18ba190186a74f30 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -27,6 +27,7 @@
> #[doc(no_inline)]
> pub use super::dbg;
> pub use super::fmt;
> +pub use super::impl_flags;
> pub use super::{dev_alert, dev_crit, dev_dbg, dev_emerg, dev_err, dev_info, dev_notice, dev_warn};
> pub use super::{pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
>
>
> ---
> base-commit: edc5e6e019c99b529b3d1f2801d5cce9924ae79b
> change-id: 20250304-feat-add-bitmask-macro-6424b1c317e2
>
> Best regards,
> --
> Filipe Xavier <felipeaggger@...il.com>
>
>
— Daniel
Powered by blists - more mailing lists