netdev - Re: [PATCH net-next v7 2/5] rust: net::phy add 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]

Date: Fri, 17 Nov 2023 14:21:38 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, andrew@...n.ch,
	tmgross@...ch.edu, miguel.ojeda.sandonis@...il.com,
	benno.lossin@...ton.me, wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 2/5] rust: net::phy add module_phy_driver
 macro

On Thu, Oct 26, 2023 at 09:10:47AM +0900, FUJITA Tomonori wrote:
[...]
> +
> +/// Declares a kernel module for PHYs drivers.
> +///
> +/// This creates a static array of kernel's `struct phy_driver` and registers it.
> +/// This also corresponds to the kernel's `MODULE_DEVICE_TABLE` macro, which embeds the information
> +/// for module loading into the module binary file. Every driver needs an entry in `device_table`.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # mod module_phy_driver_sample {
> +/// use kernel::c_str;
> +/// use kernel::net::phy::{self, DeviceId};
> +/// use kernel::prelude::*;
> +///
> +/// kernel::module_phy_driver! {
> +///     drivers: [PhyAX88772A],
> +///     device_table: [
> +///         DeviceId::new_with_driver::<PhyAX88772A>()
> +///     ],
> +///     name: "rust_asix_phy",
> +///     author: "Rust for Linux Contributors",
> +///     description: "Rust Asix PHYs driver",
> +///     license: "GPL",
> +/// }
> +///
> +/// struct PhyAX88772A;
> +///
> +/// #[vtable]
> +/// impl phy::Driver for PhyAX88772A {
> +///     const NAME: &'static CStr = c_str!("Asix Electronics AX88772A");
> +///     const PHY_DEVICE_ID: phy::DeviceId = phy::DeviceId::new_with_exact_mask(0x003b1861);
> +/// }
> +/// # }
> +/// ```

When run the following kunit command:

./tools/testing/kunit/kunit.py run --make_options LLVM=1 --arch x86_64 \
	--kconfig_add CONFIG_RUST=y \
	--kconfig_add CONFIG_RUST_PHYLIB_ABSTRACTIONS=y \
	--kconfig_add CONFIG_PHYLIB=y \
	--kconfig_add CONFIG_NETDEVICES=y \
	--kconfig_add CONFIG_NET=y \
	--kconfig_add CONFIG_AX88796B_RUST_PHY=y \
	--kconfig_add CONFIG_AX88796B_PHY=y

I got the following errors:

	ERROR:root:ld.lld: error: duplicate symbol: __rust_asix_phy_init
	>>> defined at doctests_kernel_generated.5ed8fd29a53cf22f-cgu.0
	>>>            rust/doctests_kernel_generated.o:(__rust_asix_phy_init) in archive vmlinux.a
	>>> defined at ax88796b_rust.37fb93aefca595fa-cgu.0
	>>>            drivers/net/phy/ax88796b_rust.o:(.text+0x160) in archive vmlinux.a

	ld.lld: error: duplicate symbol: __rust_asix_phy_exit
	>>> defined at doctests_kernel_generated.5ed8fd29a53cf22f-cgu.0
	>>>            rust/doctests_kernel_generated.o:(__rust_asix_phy_exit) in archive vmlinux.a
	>>> defined at ax88796b_rust.37fb93aefca595fa-cgu.0
	>>>            drivers/net/phy/ax88796b_rust.o:(.text+0x1E0) in archive vmlinux.a

	ld.lld: error: duplicate symbol: __mod_mdio__phydev_device_table
	>>> defined at doctests_kernel_generated.5ed8fd29a53cf22f-cgu.0
	>>>            rust/doctests_kernel_generated.o:(__mod_mdio__phydev_device_table) in archive vmlinux.a
	>>> defined at ax88796b_rust.37fb93aefca595fa-cgu.0
	>>>            drivers/net/phy/ax88796b_rust.o:(.rodata+0x58) in archive vmlinux.a

Because kunit will use the above doc test to generate test, and since
CONFIG_AX88796B_RUST_PHY is also selected, the `module_phy_driver!` has
been called twice, and causes duplicate symbols.

For "rust_asix_phy_*" symbols, it's easy to fix: just rename the usage
in the example. But for __mod_mdio__phydev_device_table, it's hard-coded
in `module_phy_driver!`, I don't have a quick fix right now. Also, does
it mean `module_phy_driver!` is only supposed to be "called" once for
the entire kernel?

Regards,
Boqun