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: <20231021.220012.2089903288409349337.fujita.tomonori@gmail.com> Date: Sat, 21 Oct 2023 22:00:12 +0900 (JST) From: FUJITA Tomonori <fujita.tomonori@...il.com> To: benno.lossin@...ton.me Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, andrew@...n.ch, miguel.ojeda.sandonis@...il.com, tmgross@...ch.edu, boqun.feng@...il.com, wedsonaf@...il.com, greg@...ah.com Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers On Sat, 21 Oct 2023 12:50:10 +0000 Benno Lossin <benno.lossin@...ton.me> wrote: > On 21.10.23 14:38, FUJITA Tomonori wrote: >> On Sat, 21 Oct 2023 12:13:32 +0000 >> Benno Lossin <benno.lossin@...ton.me> wrote: >> >>>>>>> Can you please share your setup and the error? For me it booted >>>>>>> fine. >>>>>> >>>>>> You use ASIX PHY hardware? >>>>> >>>>> It seems I have configured something wrong. Can you share your testing >>>>> setup? Do you use a virtual PHY device in qemu, or do you boot it from >>>>> real hardware with a real ASIX PHY device? >>>> >>>> real hardware with real ASIX PHY device. >>> >>> I see. >>> >>>> Qemu supports a virtual PHY device? >>> >>> I have no idea. >> >> When I had a look at Qemu several months ago, it didn't support such. >> >>> [...] >>> >>>>> I think this is very weird, do you have any idea why this >>>>> could happen? >>>> >>>> DriverVtable is created on kernel stack, I guess. >>> >>> But how does that invalidate the function pointers? >> >> Not only funciton pointers. You can't store something on stack for >> later use. > > It is not stored on the stack, it is only created on the stack and > moved to a global static later on. The `module!` macro creates a > `static mut __MOD: Option<Module>` where the module data is stored in. I know. The problem is that we call phy_drivers_register() with DriverVTable on stack. Then it was moved. > It seems that constructing the driver table not at that location > is somehow interfering with something? > > Wedson has a patch [1] to create in-place initialized modules, but > it probably is not completely finished, as he has not yet begun to > post it to the list. But I am sure that it is mature enough for > you to test this hypothesis. > > [1]: https://github.com/wedsonaf/linux/commit/484ec70025ff9887d9ca228ec631264039cee355 > > -- > Cheers, > Benno > >>>>> If you don't mind, could you try if the following changes >>>>> anything? >>>> >>>> I don't think it works. If you use const for DriverTable, DriverTable >>>> is placed on read-only pages. The C side modifies DriverVTable array >>>> so it does't work. >>> >>> Did you try it? Note that I copy the `DriverVTable` into the Module >>> struct, so it will not be placed on a read-only page. >> >> Ah, I misunderstood code. It doesn't work. DriverVTable on stack. >> >> >>>>> (drivers: [$($driver:ident),+], device_table: [$($dev:expr),+], $($f:tt)*) => { >>>>> const N: usize = $crate::module_phy_driver!(@count_devices $($driver),+); >>>>> struct Module { >>>>> _drivers: [::kernel::net::phy::DriverVTable; N], >>>>> } >>>>> >>>>> $crate::prelude::module! { >>>>> type: Module, >>>>> $($f)* >>>>> } >>>>> >>>>> unsafe impl Sync for Module {} >>>>> >>>>> impl ::kernel::Module for Module { >>>>> fn init(module: &'static ThisModule) -> Result<Self> { >>>>> const DRIVERS: [::kernel::net::phy::DriverVTable; N] = [$(::kernel::net::phy::create_phy_driver::<$driver>()),+]; >>>>> let mut m = Module { >>>>> _drivers: unsafe { core::ptr::read(&DRIVERS) }, >>>>> }; >>>>> let ptr = m._drivers.as_mut_ptr().cast::<::kernel::bindings::phy_driver>(); >>>>> ::kernel::error::to_result(unsafe { >>>>> kernel::bindings::phy_drivers_register(ptr, m._drivers.len().try_into()?, module.as_ptr()) >>>>> })?; >>>>> Ok(m) >>>>> } >>>>> } >>>>> >>>>> and also the variation where you replace `const DRIVERS` with >>>>> `static DRIVERS`. >>>> >>>> Probably works. But looks like similar with the current code? This is >>>> simpler? >>> >>> Just curious if it has to do with using `static` vs `const`. >> >> static doesn't work too due to the same reason. > > >
Powered by blists - more mailing lists