linux-kernel - Re: [PATCH v5 1/4] rust: debugfs: Bind DebugFS directory creation

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: <57ddf59f8f2ca740b11650360ea7d5356dad7112.camel@nvidia.com>
Date: Wed, 7 May 2025 18:46:03 +0000
From: Timur Tabi <ttabi@...dia.com>
To: "tmgross@...ch.edu" <tmgross@...ch.edu>, "benno.lossin@...ton.me"
	<benno.lossin@...ton.me>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>, "gary@...yguo.net" <gary@...yguo.net>,
	"mmaurer@...gle.com" <mmaurer@...gle.com>, "a.hindborg@...nel.org"
	<a.hindborg@...nel.org>, "bjorn3_gh@...tonmail.com"
	<bjorn3_gh@...tonmail.com>, "boqun.feng@...il.com" <boqun.feng@...il.com>,
	"dakr@...nel.org" <dakr@...nel.org>, "alex.gaynor@...il.com"
	<alex.gaynor@...il.com>, "aliceryhl@...gle.com" <aliceryhl@...gle.com>,
	"ojeda@...nel.org" <ojeda@...nel.org>, "rafael@...nel.org"
	<rafael@...nel.org>, "samitolvanen@...gle.com" <samitolvanen@...gle.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"rust-for-linux@...r.kernel.org" <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v5 1/4] rust: debugfs: Bind DebugFS directory creation

On Mon, 2025-05-05 at 23:51 +0000, Matthew Maurer wrote:
> 
> +impl<'a> Entry<'a> {
> +    /// Constructs a new DebugFS [`Entry`] from the underlying pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The pointer must either be an error code, `NULL`, or represent a transfer of ownership of
> a
> +    /// live DebugFS directory. If this is a child directory or file, `'a` must be less than the
> +    /// lifetime of the parent directory.
> +    #[cfg(CONFIG_DEBUG_FS)]
> +    unsafe fn from_ptr(entry: *mut bindings::dentry) -> Self {
> +        Self {
> +            entry,
> +            _phantom: PhantomData,
> +        }
> +    }
> +
> +    #[cfg(not(CONFIG_DEBUG_FS))]
> +    fn new() -> Self {
> +        Self {
> +            _phantom: PhantomData,
> +        }
> +    }

I am new to Rust, so forgive me if this is a dumb question, but it looks to me that if
CONFIG_DEBUG_FS is defined, then you need to call from_ptr() to create a new Entry, but if
CONFIG_DEBUG_FS is not defined, then you need to call new() instead.  Is that right?  If so, is that
really idiomatic?

In the Dir implementation below, you are careful to call from_ptr() only from the CONFIG_DEBUG_FS
version of create(), and you call new() only from the !CONFIG_DEBUG_FS version of create().  So your
bases are covered as long as no driver tries to create an Entry from scratch. 

But I guess that can't happen because Entry is not public, right?

> +    /// Create a DebugFS subdirectory.
> +    ///
> +    /// Subdirectory handles cannot outlive the directory handle they were created from.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let parent = Dir::new(c_str!("parent"));
> +    /// let child = parent.subdir(c_str!("child"));
> +    /// ```
> +    pub fn subdir<'b>(&'b self, name: &CStr) -> Dir<'b> {
> +        Dir::create(name, Some(self))
> +    }
> +
> +    /// Create a new directory in DebugFS at the root.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::c_str;
> +    /// # use kernel::debugfs::Dir;
> +    /// let debugfs = Dir::new(c_str!("parent"));
> +    /// ```
> +    pub fn new(name: &CStr) -> Self {
> +        Dir::create(name, None)
> +    }

Is there any real value to having two constructors, just to avoid passing None for the one time that
a root directory will be created?  The C code has no problem passing NULL.