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: <66455d50-9a3c-4b5c-ba2c-5188dae247a9@lunn.ch> Date: Fri, 17 Nov 2023 23:54:09 +0100 From: Andrew Lunn <andrew@...n.ch> To: Boqun Feng <boqun.feng@...il.com> Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, 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 Fri, Nov 17, 2023 at 02:21:38PM -0800, Boqun Feng wrote: > 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? Each kernel module should be in its own symbol name space. The only symbols which are visible outside of the module are those exported using EXPORT_SYMBOL_GPL() or EXPORT_SYMBOL(). A PHY driver does not export anything, in general. Being built in also does not change this. Neither drivers/net/phy/ax88796b_rust.o nor rust/doctests_kernel_generated.o should have exported this symbol. I've no idea how this actually works, i guess there are multiple passes through the linker? Maybe once to resolve symbols across object files within a module. Normal global symbols are then made local, leaving only those exported with EXPORT_SYMBOL_GPL() or EXPORT_SYMBOL()? A second pass through linker then links all the exported symbols thorough the kernel? Andrew
Powered by blists - more mailing lists