netdev - Re: [PATCH net] rust: net::phy scope ThisModule usage in the module_phy

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: <20241124162700.4ec4b6ce@kernel.org>
Date: Sun, 24 Nov 2024 16:27:00 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Rahul Rameshbabu <sergeantsagara@...tonmail.com>,
 rust-for-linux@...r.kernel.org
Cc: netdev@...r.kernel.org, FUJITA Tomonori <fujita.tomonori@...il.com>,
 Trevor Gross <tmgross@...ch.edu>, 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>, Andrew
 Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] rust: net::phy scope ThisModule usage in the
 module_phy_driver macro

On Wed, 13 Nov 2024 17:45:22 +0000 Rahul Rameshbabu wrote:
> Similar to the use of $crate::Module, ThisModule should be referred to as
> $crate::ThisModule in the macro evaluation. The reason the macro previously
> did not cause any errors is because all the users of the macro would use
> kernel::prelude::*, bringing ThisModule into scope.

You say "previously", does it mean there are no in-tree users where
this could cause bugs? If so no Fixes tag necessary..

> Fixes: 2fe11d5ab35d ("rust: net::phy add module_phy_driver macro")
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
> ---
> 
> Notes:
>     How I came up with this change:
>     
>     I was working on my own rust bindings and rust driver when I compared my
>     macro_rule to the one used for module_phy_driver. I noticed, if I made a
>     driver that does not use kernel::prelude::*, that the ThisModule type
>     identifier used in the macro would cause an error without being scoped in
>     the macro_rule. I believe the correct implementation for the macro is one
>     where the types used are correctly expanded with needed scopes.

Rust experts, does the patch itself make sense?

> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index 910ce867480a..80f9f571b88c 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -837,7 +837,7 @@ const fn as_int(&self) -> u32 {
>  ///         [::kernel::net::phy::create_phy_driver::<PhySample>()];
>  ///
>  ///     impl ::kernel::Module for Module {
> -///         fn init(module: &'static ThisModule) -> Result<Self> {
> +///         fn init(module: &'static ::kernel::ThisModule) -> Result<Self> {
>  ///             let drivers = unsafe { &mut DRIVERS };
>  ///             let mut reg = ::kernel::net::phy::Registration::register(
>  ///                 module,
> @@ -899,7 +899,7 @@ struct Module {
>                  [$($crate::net::phy::create_phy_driver::<$driver>()),+];
>  
>              impl $crate::Module for Module {
> -                fn init(module: &'static ThisModule) -> Result<Self> {
> +                fn init(module: &'static $crate::ThisModule) -> Result<Self> {
>                      // SAFETY: The anonymous constant guarantees that nobody else can access
>                      // the `DRIVERS` static. The array is used only in the C side.
>                      let drivers = unsafe { &mut DRIVERS };
> 
> base-commit: 73af53d82076bbe184d9ece9e14b0dc8599e6055