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: <D9VPJFJ44PUP.3D0HGIEJC9UGY@kernel.org>
Date: Wed, 14 May 2025 09:33:05 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Matthew Maurer" <mmaurer@...gle.com>, "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>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 "Sami Tolvanen" <samitolvanen@...gle.com>, "Timur Tabi" <ttabi@...dia.com>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v5 1/4] rust: debugfs: Bind DebugFS directory creation

On Tue May 6, 2025 at 1:51 AM CEST, Matthew Maurer wrote:
> +impl<'a> Dir<'a> {
> +    /// Create a new directory in DebugFS. If `parent` is [`None`], it will be created at the root.
> +    #[cfg(CONFIG_DEBUG_FS)]
> +    fn create<'b>(name: &CStr, parent: Option<&'a Dir<'b>>) -> Self {
> +        let parent_ptr = match parent {
> +            Some(parent) => parent.0.as_ptr(),
> +            None => core::ptr::null_mut(),
> +        };
> +        // SAFETY:
> +        // * `name` argument points to a NUL-terminated string that lives across the call, by
> +        //   invariants of `&CStr`.
> +        // * If `parent` is `None`, `parent` accepts null pointers to mean create at root.
> +        // * If `parent` is `Some`, `parent` accepts live dentry debugfs pointers.
> +        //   so we can call `Self::from_ptr`.
> +        let dir = unsafe { bindings::debugfs_create_dir(name.as_char_ptr(), parent_ptr) };
> +
> +        // SAFETY: `debugfs_create_dir` either returns an error code or a legal `dentry` pointer,
> +        Self(unsafe { Entry::from_ptr(dir) })
> +    }
> +
> +    #[cfg(not(CONFIG_DEBUG_FS))]
> +    fn create<'b>(_name: &CStr, _parent: Option<&'a Dir<'b>>) -> Self {
> +        Self(Entry::new())
> +    }
> +
> +    /// Create a DebugFS subdirectory.

I'm not familiar with debugfs, if I run `Dir::create(c"foo", None)`
twice, will both of the returned values refer to the same or different
directories? What if I give a parent?
If the answer in both cases is that they will refer to the same
directory, then I'd change the docs to mention that. So instead of
"Creates" we could say "Finds or creates" or something better.
If they refer to different files, then I am confused how that would look
like in user-land :)

> +    ///
> +    /// 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.

Ditto here.

> +    ///
> +    /// # 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)
> +    }

I think it would make more sense for this function to return
`Dir<'static>`.

---
Cheers,
Benno

> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c3762e80b314316b4b0cee3bfd9442f8f0510b91..86f6055b828d5f711578293d8916a517f2436977 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -45,6 +45,7 @@
>  #[doc(hidden)]
>  pub mod build_assert;
>  pub mod cred;
> +pub mod debugfs;
>  pub mod device;
>  pub mod device_id;
>  pub mod devres;