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: <77c78010-781e-4eb4-a7ba-3e9f9a07bf67@proton.me> Date: Fri, 27 Oct 2023 22:50:45 +0000 From: Benno Lossin <benno.lossin@...ton.me> To: Boqun Feng <boqun.feng@...il.com> Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org, rust-for-linux@...r.kernel.org, andrew@...n.ch, tmgross@...ch.edu, miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers On 10/28/23 00:21, Boqun Feng wrote: > On Fri, Oct 27, 2023 at 09:19:38PM +0000, Benno Lossin wrote: >> We need to be careful here, since doing this creates a reference >> `&bindings::phy_device` which asserts that it is immutable. That is not >> the case, since the C side might change it at any point (this is the >> reason we wrap things in `Opaque`, since that allows mutatation even >> through sharde references). >> >> I did not notice this before, but this means we cannot use the `link` >> function from bindgen, since that takes `&self`. We would need a >> function that takes `*const Self` instead. >> > > Hmm... but does it mean even `set_speed()` has the similar issue? > > let phydev: *mut phy_device = self.0.get(); > unsafe { (*phydev).speed = ...; } No that should be fine, take a look at the MIR output of the following code [1]: struct Foo { a: usize, b: usize, } fn foo(ptr: *mut Foo) { unsafe { (*ptr).b = 0; } } fn bar(ptr: *mut Foo) { unsafe { (&mut *ptr).b = 0; } } Aside from some alignment checking, foo's MIR looks like this: bb1: { ((*_1).1: usize) = const 0_usize; return; } And bar's MIR like this: bb1: { _2 = &mut (*_1); ((*_2).1: usize) = const 0_usize; return; } [1]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f7c4d87bf29a64af0acc09ff75d3716d So I think that is fine, but maybe Gary has something else to say about it. > The `(*phydev)` creates a `&mut` IIUC. So we need the following maybe? > > let phydev: *mut phy_device = self.0.get(); > unsafe { *addr_mut_of!((*phydev).speed) = ...; } > > because at least from phylib core guarantee, we know no one accessing > `speed` in the same time. However, yes, bit fields are tricky... -- Cheers, Benno
Powered by blists - more mailing lists