| 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: <CAJ-ks9=FauL4LGMhrdDbd-NSaxHQ=w_ZtcntSuz2ZgSJWx2wvw@mail.gmail.com>
Date: Fri, 18 Apr 2025 11:15:49 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Boqun Feng <boqun.feng@...il.com>
Cc: Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>, Brendan Higgins <brendan.higgins@...ux.dev>,
David Gow <davidgow@...gle.com>, Rae Moar <rmoar@...gle.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, Luis Chamberlain <mcgrof@...nel.org>,
Russ Weight <russ.weight@...ux.dev>, Rob Herring <robh@...nel.org>,
Saravana Kannan <saravanak@...gle.com>, Abdiel Janulgue <abdiel.janulgue@...il.com>,
Daniel Almeida <daniel.almeida@...labora.com>, Robin Murphy <robin.murphy@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
FUJITA Tomonori <fujita.tomonori@...il.com>, Nicolas Schier <nicolas.schier@...ux.dev>,
Frederic Weisbecker <frederic@...nel.org>, Lyude Paul <lyude@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>, Anna-Maria Behnsen <anna-maria@...utronix.de>,
linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org, linux-kselftest@...r.kernel.org,
kunit-dev@...glegroups.com, linux-pci@...r.kernel.org,
linux-block@...r.kernel.org, devicetree@...r.kernel.org,
dri-devel@...ts.freedesktop.org, netdev@...r.kernel.org
Subject: Re: [PATCH v9 4/6] rust: enable `clippy::as_underscore` lint
On Fri, Apr 18, 2025 at 11:09 AM Boqun Feng <boqun.feng@...il.com> wrote:
>
> On Fri, Apr 18, 2025 at 08:08:02AM -0400, Tamir Duberstein wrote:
> > On Thu, Apr 17, 2025 at 4:12 PM Boqun Feng <boqun.feng@...il.com> wrote:
> > >
> > > On Thu, Apr 17, 2025 at 03:26:14PM -0400, Tamir Duberstein wrote:
> > > [...]
> > > > >
> > > > > > Ok(())
> > > > > > }
> > > > > > diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> > > > > > index e5859217a579..4063f09d76d9 100644
> > > > > > --- a/rust/kernel/device_id.rs
> > > > > > +++ b/rust/kernel/device_id.rs
> > > > > > @@ -82,7 +82,7 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
> > > > > > unsafe {
> > > > > > raw_ids[i]
> > > > > > .as_mut_ptr()
> > > > > > - .byte_offset(T::DRIVER_DATA_OFFSET as _)
> > > > > > + .byte_add(T::DRIVER_DATA_OFFSET)
> > > > > > .cast::<usize>()
> > > > > > .write(i);
> > > > > > }
> > > > > > diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> > > > > > index f7e8f5f53622..70d12014e476 100644
> > > > > > --- a/rust/kernel/devres.rs
> > > > > > +++ b/rust/kernel/devres.rs
> > > > > > @@ -45,7 +45,7 @@ struct DevresInner<T> {
> > > > > > /// # Example
> > > > > > ///
> > > > > > /// ```no_run
> > > > > > -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
> > > > > > +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
> > > > > > /// # use core::ops::Deref;
> > > > > > ///
> > > > > > /// // See also [`pci::Bar`] for a real example.
> > > > > > @@ -59,19 +59,19 @@ struct DevresInner<T> {
> > > > > > /// unsafe fn new(paddr: usize) -> Result<Self>{
> > > > > > /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> > > > > > /// // valid for `ioremap`.
> > > > > > -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
> > > > > > +/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
> > > > >
> > > > >
> > > > > /// let addr = unsafe { bindings::ioremap(bindings::phys_addr_t::from(paddr), SIZE) };
> > > > >
> > > > > better? Or even with .into()
> > > > >
> > > > > /// let addr = unsafe { bindings::ioremap(paddr.into(), SIZE) };
> > > >
> > > > This doesn't compile because `paddr` is usize, and
> > > > `bindings::phys_addr_t` is u64 (on my machine, which is aarch64).
> > > >
> > >
> > > Ok, looks like Rust yet doesn't provide From/Into between usize and u64
> > > even if the pointer size is fixed. Latest discussion can be found at:
> > >
> > > https://github.com/rust-lang/rust/issues/41619#issuecomment-2056902943
> > >
> > > Lemme see if we can get an issue tracking this. Thanks for taking a
> > > look.
> > >
> > > > > > /// if addr.is_null() {
> > > > > > /// return Err(ENOMEM);
> > > > > > /// }
> > > > > > ///
> > > > > > -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
> > > > > > +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
> > > > > > /// }
> > > > > > /// }
> > > > > > ///
> > > > > > /// impl<const SIZE: usize> Drop for IoMem<SIZE> {
> > > > > > /// fn drop(&mut self) {
> > > > > > /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
> > > > > > -/// unsafe { bindings::iounmap(self.0.addr() as _); };
> > > > > > +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
> > > > > > /// }
> > > > > > /// }
> > > > > > ///
> > > > > [...]
> > > > > > diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> > > > > > index 43ecf3c2e860..851a6339aa90 100644
> > > > > > --- a/rust/kernel/dma.rs
> > > > > > +++ b/rust/kernel/dma.rs
> > > > > > @@ -38,7 +38,7 @@
> > > > > > impl Attrs {
> > > > > > /// Get the raw representation of this attribute.
> > > > > > pub(crate) fn as_raw(self) -> crate::ffi::c_ulong {
> > > > > > - self.0 as _
> > > > > > + self.0 as crate::ffi::c_ulong
> > > > >
> > > > > crate::ffi::c_ulong::from(self.0)
> > > > >
> > > > > maybe, a C unsigned long should always be able to hold the whole `Attr`
> > > > > and a lossly casting is what this function does.
> > > >
> > > > This also doesn't compile: "the trait `core::convert::From<u32>` is
> > > > not implemented for `usize`". Upstream has ambitions of running on
> > > > 16-bit, I guess :)
> > > >
> > >
> > > They do but they also have the target_pointer_width cfg, so they can
> > > totally provide these functions. It's just they want to find a better
> > > way (like the link I post above).
> >
> > Did you want me to hold off on the respin on this point, or shall I go ahead?
>
> No need to wait. This doesn't affect your current patch. But we do need
> to start making some decisions about how we handle the conversions *32
> => *size, *size => *64 and c*long <=> *size, they should all be lossless
> in the context of kernel. We have 3 options:
>
> 1. Using `as`.
> 2. Having our own to_*size(*32), to_*64(*size), to_*size(*64),
> to_c*long(*size) and to_*size(c*long) helper functions.
> 3. Waiting and using what Rust upstream comes up.
>
> I'm leaning towards to 2 and then 3, because using `as` can sometimes
> surprise you when you change the type. Thoughts?
Yes, I think that's the right thing to do. That, along with the
provenance functions, might even allow us to turn on
https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions.
Powered by blists - more mailing lists