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: <23348649-2ef2-4b2d-9745-86587a72ae5e@proton.me> Date: Sat, 21 Oct 2023 12:50:10 +0000 From: Benno Lossin <benno.lossin@...ton.me> To: FUJITA Tomonori <fujita.tomonori@...il.com> Cc: 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 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. 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