Makefile | 4 ++ init/Kconfig | 3 + rust/bindings/lib.rs | 1 + rust/kernel/alloc.rs | 2 +- rust/kernel/alloc/allocator_test.rs | 2 +- rust/kernel/alloc/kvec.rs | 4 +- rust/kernel/block/mq/operations.rs | 2 +- rust/kernel/block/mq/request.rs | 7 +- rust/kernel/device.rs | 5 +- rust/kernel/device_id.rs | 2 +- rust/kernel/devres.rs | 19 +++--- rust/kernel/error.rs | 2 +- rust/kernel/firmware.rs | 3 +- rust/kernel/fs/file.rs | 2 +- rust/kernel/io.rs | 16 ++--- rust/kernel/kunit.rs | 15 ++--- rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++- rust/kernel/list/impl_list_item_mod.rs | 2 +- rust/kernel/miscdevice.rs | 2 +- rust/kernel/of.rs | 6 +- rust/kernel/pci.rs | 15 +++-- rust/kernel/platform.rs | 6 +- rust/kernel/print.rs | 11 ++-- rust/kernel/rbtree.rs | 23 +++---- rust/kernel/seq_file.rs | 3 +- rust/kernel/str.rs | 18 ++---- rust/kernel/sync/poll.rs | 2 +- rust/kernel/uaccess.rs | 12 ++-- rust/kernel/workqueue.rs | 12 ++-- rust/uapi/lib.rs | 1 + 30 files changed, 218 insertions(+), 97 deletions(-)
This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
Lossin suggested I also look into `clippy::ptr_cast_constness` and I
discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
lints. It also enables `clippy::as_underscore` which ensures other
pointer casts weren't missed. The first commit reduces the need for
pointer casts and is shared with another series[1].
The final patch also enables pointer provenance lints and fixes
violations. See that commit message for details. The build system
portion of that commit is pretty messy but I couldn't find a better way
to convincingly ensure that these lints were applied globally.
Suggestions would be very welcome.
Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v5:
- Use `pointer::addr` in OF. (Boqun Feng)
- Add documentation on stubs. (Benno Lossin)
- Mark stubs `#[inline]`.
- Pick up Alice's RB on a shared commit from
https://lore.kernel.org/all/Z9f-3Aj3_FWBZRrm@google.com/.
- Link to v4: https://lore.kernel.org/r/20250315-ptr-as-ptr-v4-0-b2d72c14dc26@gmail.com
Changes in v4:
- Add missing SoB. (Benno Lossin)
- Use `without_provenance_mut` in alloc. (Boqun Feng)
- Limit strict provenance lints to the `kernel` crate to avoid complex
logic in the build system. This can be revisited on MSRV >= 1.84.0.
- Rebase on rust-next.
- Link to v3: https://lore.kernel.org/r/20250314-ptr-as-ptr-v3-0-e7ba61048f4a@gmail.com
Changes in v3:
- Fixed clippy warning in rust/kernel/firmware.rs. (kernel test robot)
Link: https://lore.kernel.org/all/202503120332.YTCpFEvv-lkp@intel.com/
- s/as u64/as bindings::phys_addr_t/g. (Benno Lossin)
- Use strict provenance APIs and enable lints. (Benno Lossin)
- Link to v2: https://lore.kernel.org/r/20250309-ptr-as-ptr-v2-0-25d60ad922b7@gmail.com
Changes in v2:
- Fixed typo in first commit message.
- Added additional patches, converted to series.
- Link to v1: https://lore.kernel.org/r/20250307-ptr-as-ptr-v1-1-582d06514c98@gmail.com
---
Tamir Duberstein (6):
rust: retain pointer mut-ness in `container_of!`
rust: enable `clippy::ptr_as_ptr` lint
rust: enable `clippy::ptr_cast_constness` lint
rust: enable `clippy::as_ptr_cast_mut` lint
rust: enable `clippy::as_underscore` lint
rust: use strict provenance APIs
Makefile | 4 ++
init/Kconfig | 3 +
rust/bindings/lib.rs | 1 +
rust/kernel/alloc.rs | 2 +-
rust/kernel/alloc/allocator_test.rs | 2 +-
rust/kernel/alloc/kvec.rs | 4 +-
rust/kernel/block/mq/operations.rs | 2 +-
rust/kernel/block/mq/request.rs | 7 +-
rust/kernel/device.rs | 5 +-
rust/kernel/device_id.rs | 2 +-
rust/kernel/devres.rs | 19 +++---
rust/kernel/error.rs | 2 +-
rust/kernel/firmware.rs | 3 +-
rust/kernel/fs/file.rs | 2 +-
rust/kernel/io.rs | 16 ++---
rust/kernel/kunit.rs | 15 ++---
rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
rust/kernel/list/impl_list_item_mod.rs | 2 +-
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/of.rs | 6 +-
rust/kernel/pci.rs | 15 +++--
rust/kernel/platform.rs | 6 +-
rust/kernel/print.rs | 11 ++--
rust/kernel/rbtree.rs | 23 +++----
rust/kernel/seq_file.rs | 3 +-
rust/kernel/str.rs | 18 ++----
rust/kernel/sync/poll.rs | 2 +-
rust/kernel/uaccess.rs | 12 ++--
rust/kernel/workqueue.rs | 12 ++--
rust/uapi/lib.rs | 1 +
30 files changed, 218 insertions(+), 97 deletions(-)
---
base-commit: 498f7ee4773f22924f00630136da8575f38954e8
change-id: 20250307-ptr-as-ptr-21b1867fc4d4
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> lints. It also enables `clippy::as_underscore` which ensures other
> pointer casts weren't missed. The first commit reduces the need for
> pointer casts and is shared with another series[1].
>
> The final patch also enables pointer provenance lints and fixes
> violations. See that commit message for details. The build system
> portion of that commit is pretty messy but I couldn't find a better way
> to convincingly ensure that these lints were applied globally.
> Suggestions would be very welcome.
I applied the patches to v6.14-rc7 and did a quick pass with
rg -nC 3 -t rust ' as ' | bat -l rust
to see if there are any cases left that we could fix and I found a
couple:
* there are several cases of `number as int_type` (like `num as c_int`
or `my_u32 as usize` etc.) not sure what we can do about these, some
are probably unavoidable, but since the kernel doesn't support 16 bit
systems (that is true, right?), we *could* have a `From<u32> for
usize` impl...
* some instances of `'|' as u32` (samples/rust/rust_misc_device.rs:112).
There is a `From<char> for u32` impl, so this can just be replaced
with `.into()` (or maybe by using a byte literal `b'|'`?).
* `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
replace with `let ptr: *const ... = shared_ref;`. Don't know if there
is a clippy lint for this.
* some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
not sure if they can be converted though (maybe they are unsizing the
pointer?)
Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
one can be replaced by a `.cast()`)
Some clippy lints that we could also enable that share the spirit of
this series:
* `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
above?)
* `cast_lossless` (maybe this catches some of the `num as int_type`
conversions I mentioned above)
I'll leave it up to you what you want to do with this: add it to this
series, make a new one, or let someone else handle it. If you don't want
to handle it, let me know, then I'll create a good-first-issue :)
> ---
> Tamir Duberstein (6):
> rust: retain pointer mut-ness in `container_of!`
> rust: enable `clippy::ptr_as_ptr` lint
> rust: enable `clippy::ptr_cast_constness` lint
> rust: enable `clippy::as_ptr_cast_mut` lint
> rust: enable `clippy::as_underscore` lint
> rust: use strict provenance APIs
>
> Makefile | 4 ++
> init/Kconfig | 3 +
> rust/bindings/lib.rs | 1 +
> rust/kernel/alloc.rs | 2 +-
> rust/kernel/alloc/allocator_test.rs | 2 +-
> rust/kernel/alloc/kvec.rs | 4 +-
> rust/kernel/block/mq/operations.rs | 2 +-
> rust/kernel/block/mq/request.rs | 7 +-
> rust/kernel/device.rs | 5 +-
> rust/kernel/device_id.rs | 2 +-
> rust/kernel/devres.rs | 19 +++---
> rust/kernel/error.rs | 2 +-
> rust/kernel/firmware.rs | 3 +-
> rust/kernel/fs/file.rs | 2 +-
> rust/kernel/io.rs | 16 ++---
> rust/kernel/kunit.rs | 15 ++---
> rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
> rust/kernel/list/impl_list_item_mod.rs | 2 +-
> rust/kernel/miscdevice.rs | 2 +-
> rust/kernel/of.rs | 6 +-
> rust/kernel/pci.rs | 15 +++--
> rust/kernel/platform.rs | 6 +-
> rust/kernel/print.rs | 11 ++--
> rust/kernel/rbtree.rs | 23 +++----
> rust/kernel/seq_file.rs | 3 +-
> rust/kernel/str.rs | 18 ++----
> rust/kernel/sync/poll.rs | 2 +-
> rust/kernel/uaccess.rs | 12 ++--
> rust/kernel/workqueue.rs | 12 ++--
> rust/uapi/lib.rs | 1 +
> 30 files changed, 218 insertions(+), 97 deletions(-)
> ---
> base-commit: 498f7ee4773f22924f00630136da8575f38954e8
Btw I didn't find this commit anywhere I usually check, where is it
from?
---
Cheers,
Benno
> change-id: 20250307-ptr-as-ptr-21b1867fc4d4
On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> > Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> > lints. It also enables `clippy::as_underscore` which ensures other
> > pointer casts weren't missed. The first commit reduces the need for
> > pointer casts and is shared with another series[1].
> >
> > The final patch also enables pointer provenance lints and fixes
> > violations. See that commit message for details. The build system
> > portion of that commit is pretty messy but I couldn't find a better way
> > to convincingly ensure that these lints were applied globally.
> > Suggestions would be very welcome.
>
> I applied the patches to v6.14-rc7 and did a quick pass with
>
> rg -nC 3 -t rust ' as ' | bat -l rust
>
> to see if there are any cases left that we could fix and I found a
> couple:
>
> * there are several cases of `number as int_type` (like `num as c_int`
> or `my_u32 as usize` etc.) not sure what we can do about these, some
> are probably unavoidable, but since the kernel doesn't support 16 bit
> systems (that is true, right?), we *could* have a `From<u32> for
> usize` impl...
Yeah, these are the most difficult ones to get rid of.
> * some instances of `'|' as u32` (samples/rust/rust_misc_device.rs:112).
> There is a `From<char> for u32` impl, so this can just be replaced
> with `.into()` (or maybe by using a byte literal `b'|'`?).
We can enable https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#cast_lossless
for this one.
> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> is a clippy lint for this.
I think there's not a focused one. There's a nuclear option:
https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> not sure if they can be converted though (maybe they are unsizing the
> pointer?)
I have a local series that gets rid of these by doing similar things
to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
I can send it later this week but it probably can't land until Alice
is back from vacation; she was the author of this code.
> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
> one can be replaced by a `.cast()`)
>
> Some clippy lints that we could also enable that share the spirit of
> this series:
>
> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
> above?)
It's already enabled, it's warn-by-default.
> * `cast_lossless` (maybe this catches some of the `num as int_type`
> conversions I mentioned above)
Yeah, suggested the same above. I had hoped this would deal with the
char as u32 pattern but it did not.
> I'll leave it up to you what you want to do with this: add it to this
> series, make a new one, or let someone else handle it. If you don't want
> to handle it, let me know, then I'll create a good-first-issue :)
I'll add a patch for `cast_lossless` -- the rest should probably go
into an issue.
>
> > ---
> > Tamir Duberstein (6):
> > rust: retain pointer mut-ness in `container_of!`
> > rust: enable `clippy::ptr_as_ptr` lint
> > rust: enable `clippy::ptr_cast_constness` lint
> > rust: enable `clippy::as_ptr_cast_mut` lint
> > rust: enable `clippy::as_underscore` lint
> > rust: use strict provenance APIs
> >
> > Makefile | 4 ++
> > init/Kconfig | 3 +
> > rust/bindings/lib.rs | 1 +
> > rust/kernel/alloc.rs | 2 +-
> > rust/kernel/alloc/allocator_test.rs | 2 +-
> > rust/kernel/alloc/kvec.rs | 4 +-
> > rust/kernel/block/mq/operations.rs | 2 +-
> > rust/kernel/block/mq/request.rs | 7 +-
> > rust/kernel/device.rs | 5 +-
> > rust/kernel/device_id.rs | 2 +-
> > rust/kernel/devres.rs | 19 +++---
> > rust/kernel/error.rs | 2 +-
> > rust/kernel/firmware.rs | 3 +-
> > rust/kernel/fs/file.rs | 2 +-
> > rust/kernel/io.rs | 16 ++---
> > rust/kernel/kunit.rs | 15 ++---
> > rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
> > rust/kernel/list/impl_list_item_mod.rs | 2 +-
> > rust/kernel/miscdevice.rs | 2 +-
> > rust/kernel/of.rs | 6 +-
> > rust/kernel/pci.rs | 15 +++--
> > rust/kernel/platform.rs | 6 +-
> > rust/kernel/print.rs | 11 ++--
> > rust/kernel/rbtree.rs | 23 +++----
> > rust/kernel/seq_file.rs | 3 +-
> > rust/kernel/str.rs | 18 ++----
> > rust/kernel/sync/poll.rs | 2 +-
> > rust/kernel/uaccess.rs | 12 ++--
> > rust/kernel/workqueue.rs | 12 ++--
> > rust/uapi/lib.rs | 1 +
> > 30 files changed, 218 insertions(+), 97 deletions(-)
> > ---
> > base-commit: 498f7ee4773f22924f00630136da8575f38954e8
>
> Btw I didn't find this commit anywhere I usually check, where is it
> from?
It was probably nowhere, a local frankenstein that included some fixes
from rust-fixes.
On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> is a clippy lint for this.
>
> I think there's not a focused one. There's a nuclear option:
> https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
Yeah I saw that one, I don't think it's a good idea, since there will be
false positives.
>> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
>> not sure if they can be converted though (maybe they are unsizing the
>> pointer?)
>
> I have a local series that gets rid of these by doing similar things
> to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> I can send it later this week but it probably can't land until Alice
> is back from vacation; she was the author of this code.
No worries, as I wrote below, I think it's fine to do that in a new
series.
>> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
>> one can be replaced by a `.cast()`)
>>
>> Some clippy lints that we could also enable that share the spirit of
>> this series:
>>
>> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
>> above?)
>
> It's already enabled, it's warn-by-default.
Ah I see, didn't look :)
>> * `cast_lossless` (maybe this catches some of the `num as int_type`
>> conversions I mentioned above)
>
> Yeah, suggested the same above. I had hoped this would deal with the
> char as u32 pattern but it did not.
Aw that's a shame. Maybe we should create a clippy issue for that,
thoughts?
>> I'll leave it up to you what you want to do with this: add it to this
>> series, make a new one, or let someone else handle it. If you don't want
>> to handle it, let me know, then I'll create a good-first-issue :)
>
> I'll add a patch for `cast_lossless` -- the rest should probably go
> into an issue.
Do you mind filing the issue? Then you can decide yourself what you want
to do yourself vs what you want to leave for others. Feel free to copy
from my mail summary.
Also I wouldn't mark it as a good-first-issue yet, since it's pretty
complicated and needs to be delayed/based on this series.
---
Cheers,
Benno
On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> >> is a clippy lint for this.
> >
> > I think there's not a focused one. There's a nuclear option:
> > https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
>
> Yeah I saw that one, I don't think it's a good idea, since there will be
> false positives.
>
> >> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> >> not sure if they can be converted though (maybe they are unsizing the
> >> pointer?)
> >
> > I have a local series that gets rid of these by doing similar things
> > to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> > I can send it later this week but it probably can't land until Alice
> > is back from vacation; she was the author of this code.
>
> No worries, as I wrote below, I think it's fine to do that in a new
> series.
>
> >> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
> >> one can be replaced by a `.cast()`)
> >>
> >> Some clippy lints that we could also enable that share the spirit of
> >> this series:
> >>
> >> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
> >> above?)
> >
> > It's already enabled, it's warn-by-default.
>
> Ah I see, didn't look :)
>
> >> * `cast_lossless` (maybe this catches some of the `num as int_type`
> >> conversions I mentioned above)
> >
> > Yeah, suggested the same above. I had hoped this would deal with the
> > char as u32 pattern but it did not.
>
> Aw that's a shame. Maybe we should create a clippy issue for that,
> thoughts?
Yeah, it's not clear to me why it isn't covered by `cast_lossless`.
Might just be a bug. Want to file it?
>
> >> I'll leave it up to you what you want to do with this: add it to this
> >> series, make a new one, or let someone else handle it. If you don't want
> >> to handle it, let me know, then I'll create a good-first-issue :)
> >
> > I'll add a patch for `cast_lossless` -- the rest should probably go
> > into an issue.
>
> Do you mind filing the issue? Then you can decide yourself what you want
> to do yourself vs what you want to leave for others. Feel free to copy
> from my mail summary.
Well, I don't really know what's left to do. We're pretty close at
this point to having enabled everything but the nukes. Then there's
the strict provenance thing, which I suppose we can write down.
> Also I wouldn't mark it as a good-first-issue yet, since it's pretty
> complicated and needs to be delayed/based on this series.
Yeah, certainly not good-first-issue.
On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote: > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote: >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote: >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote: >> >> * `cast_lossless` (maybe this catches some of the `num as int_type` >> >> conversions I mentioned above) >> > >> > Yeah, suggested the same above. I had hoped this would deal with the >> > char as u32 pattern but it did not. >> >> Aw that's a shame. Maybe we should create a clippy issue for that, >> thoughts? > > Yeah, it's not clear to me why it isn't covered by `cast_lossless`. > Might just be a bug. Want to file it? Done: https://github.com/rust-lang/rust-clippy/issues/14469 >> >> I'll leave it up to you what you want to do with this: add it to this >> >> series, make a new one, or let someone else handle it. If you don't want >> >> to handle it, let me know, then I'll create a good-first-issue :) >> > >> > I'll add a patch for `cast_lossless` -- the rest should probably go >> > into an issue. >> >> Do you mind filing the issue? Then you can decide yourself what you want >> to do yourself vs what you want to leave for others. Feel free to copy >> from my mail summary. > > Well, I don't really know what's left to do. We're pretty close at > this point to having enabled everything but the nukes. Then there's > the strict provenance thing, which I suppose we can write down. Yes, but there are also these from my original mail: * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247, rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can replace with `let ptr: *const ... = shared_ref;`. Don't know if there is a clippy lint for this. And the other points (haven't taken a look at the other series you submitted, so I don't know to what extend you fixed the other `as` casts I mentioned). So I figured you might know which ones we still have after applying all your patches :) --- Cheers, Benno
On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote: > > On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote: > > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote: > >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote: > >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote: > >> >> * `cast_lossless` (maybe this catches some of the `num as int_type` > >> >> conversions I mentioned above) > >> > > >> > Yeah, suggested the same above. I had hoped this would deal with the > >> > char as u32 pattern but it did not. > >> > >> Aw that's a shame. Maybe we should create a clippy issue for that, > >> thoughts? > > > > Yeah, it's not clear to me why it isn't covered by `cast_lossless`. > > Might just be a bug. Want to file it? > > Done: https://github.com/rust-lang/rust-clippy/issues/14469 Nice, looks like there's already a PR out: https://github.com/rust-lang/rust-clippy/pull/14470. > >> >> I'll leave it up to you what you want to do with this: add it to this > >> >> series, make a new one, or let someone else handle it. If you don't want > >> >> to handle it, let me know, then I'll create a good-first-issue :) > >> > > >> > I'll add a patch for `cast_lossless` -- the rest should probably go > >> > into an issue. > >> > >> Do you mind filing the issue? Then you can decide yourself what you want > >> to do yourself vs what you want to leave for others. Feel free to copy > >> from my mail summary. > > > > Well, I don't really know what's left to do. We're pretty close at > > this point to having enabled everything but the nukes. Then there's > > the strict provenance thing, which I suppose we can write down. > > Yes, but there are also these from my original mail: > * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247, > rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can > replace with `let ptr: *const ... = shared_ref;`. Don't know if there > is a clippy lint for this. I don't think we should go fixing things for which we don't have a clippy lint. That way lies madness, particularly as patches begin to be carried by other trees. > > And the other points (haven't taken a look at the other series you > submitted, so I don't know to what extend you fixed the other `as` casts > I mentioned). So I figured you might know which ones we still have after > applying all your patches :) > > --- > Cheers, > Benno >
On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
>> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
>> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> I'll leave it up to you what you want to do with this: add it to this
>> >> >> series, make a new one, or let someone else handle it. If you don't want
>> >> >> to handle it, let me know, then I'll create a good-first-issue :)
>> >> >
>> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
>> >> > into an issue.
>> >>
>> >> Do you mind filing the issue? Then you can decide yourself what you want
>> >> to do yourself vs what you want to leave for others. Feel free to copy
>> >> from my mail summary.
>> >
>> > Well, I don't really know what's left to do. We're pretty close at
>> > this point to having enabled everything but the nukes. Then there's
>> > the strict provenance thing, which I suppose we can write down.
>>
>> Yes, but there are also these from my original mail:
>> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> is a clippy lint for this.
>
> I don't think we should go fixing things for which we don't have a
> clippy lint. That way lies madness, particularly as patches begin to
> be carried by other trees.
There already exists a lint for that: `clippy::ref_as_ptr` (almost
created an issue asking for one :)
Here is another lint that we probably want to enable (after the `&raw
{const,mut}` series lands): `clippy::borrow_as_ptr`.
---
Cheers,
Benno
On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
> > On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
> >> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> >> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> >> I'll leave it up to you what you want to do with this: add it to this
> >> >> >> series, make a new one, or let someone else handle it. If you don't want
> >> >> >> to handle it, let me know, then I'll create a good-first-issue :)
> >> >> >
> >> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
> >> >> > into an issue.
> >> >>
> >> >> Do you mind filing the issue? Then you can decide yourself what you want
> >> >> to do yourself vs what you want to leave for others. Feel free to copy
> >> >> from my mail summary.
> >> >
> >> > Well, I don't really know what's left to do. We're pretty close at
> >> > this point to having enabled everything but the nukes. Then there's
> >> > the strict provenance thing, which I suppose we can write down.
> >>
> >> Yes, but there are also these from my original mail:
> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> >> is a clippy lint for this.
> >
> > I don't think we should go fixing things for which we don't have a
> > clippy lint. That way lies madness, particularly as patches begin to
> > be carried by other trees.
>
> There already exists a lint for that: `clippy::ref_as_ptr` (almost
> created an issue asking for one :)
🫡 picked this one up as well.
> Here is another lint that we probably want to enable (after the `&raw
> {const,mut}` series lands): `clippy::borrow_as_ptr`.
This sounds like a good one to file.
On Tue Mar 25, 2025 at 6:17 PM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
>> > On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
>> >> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
>> >> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> >> I'll leave it up to you what you want to do with this: add it to this
>> >> >> >> series, make a new one, or let someone else handle it. If you don't want
>> >> >> >> to handle it, let me know, then I'll create a good-first-issue :)
>> >> >> >
>> >> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
>> >> >> > into an issue.
>> >> >>
>> >> >> Do you mind filing the issue? Then you can decide yourself what you want
>> >> >> to do yourself vs what you want to leave for others. Feel free to copy
>> >> >> from my mail summary.
>> >> >
>> >> > Well, I don't really know what's left to do. We're pretty close at
>> >> > this point to having enabled everything but the nukes. Then there's
>> >> > the strict provenance thing, which I suppose we can write down.
>> >>
>> >> Yes, but there are also these from my original mail:
>> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> >> is a clippy lint for this.
>> >
>> > I don't think we should go fixing things for which we don't have a
>> > clippy lint. That way lies madness, particularly as patches begin to
>> > be carried by other trees.
>>
>> There already exists a lint for that: `clippy::ref_as_ptr` (almost
>> created an issue asking for one :)
>
> 🫡 picked this one up as well.
Sniped yet again :)
Thanks a lot for adding this one as well.
>> Here is another lint that we probably want to enable (after the `&raw
>> {const,mut}` series lands): `clippy::borrow_as_ptr`.
>
> This sounds like a good one to file.
Since `raw_ref_op` is already enabled in rust-next, I just filed it:
https://github.com/Rust-for-Linux/linux/issues/1152
---
Cheers,
Benno
On Tue, Mar 25, 2025 at 1:17 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> > Here is another lint that we probably want to enable (after the `&raw
> > {const,mut}` series lands): `clippy::borrow_as_ptr`.
>
> This sounds like a good one to file.
Actually I just enabled this and it has no incremental effect relative
to the lints already enabled in the series. We can enable it now, but
it seems to only trigger on patterns already caught by `ref_as_ptr`.
On Tue, Mar 25, 2025 at 12:05 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Done: https://github.com/rust-lang/rust-clippy/issues/14469
Linked in our list:
https://github.com/Rust-for-Linux/linux/issues/349
Cheers,
Miguel
On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
>
> > * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> > not sure if they can be converted though (maybe they are unsizing the
> > pointer?)
>
> I have a local series that gets rid of these by doing similar things
> to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> I can send it later this week but it probably can't land until Alice
> is back from vacation; she was the author of this code.
https://lore.kernel.org/all/20250324-list-no-offset-v1-0-afd2b7fc442a@gmail.com/
On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote: > > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote: > > > > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote: > > > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno > > > Lossin suggested I also look into `clippy::ptr_cast_constness` and I > > > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3 > > > lints. It also enables `clippy::as_underscore` which ensures other > > > pointer casts weren't missed. The first commit reduces the need for > > > pointer casts and is shared with another series[1]. > > > > > > The final patch also enables pointer provenance lints and fixes > > > violations. See that commit message for details. The build system > > > portion of that commit is pretty messy but I couldn't find a better way > > > to convincingly ensure that these lints were applied globally. > > > Suggestions would be very welcome. > > > > I applied the patches to v6.14-rc7 and did a quick pass with So I rebased this on rust-next and fixed a few more instances (in addition to enabling the extra lint), but I realized that rust-next is still based on v6.14-rc5. I think we're going to have the same problem here as in the &raw series; either Miguel is going to have to apply fixups when picking these patches, or we have to split the fixes out from the lints and land it over several cycles. Thoughts?
On Mon Mar 24, 2025 at 10:16 PM CET, Tamir Duberstein wrote: > On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote: >> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote: >> > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote: >> > > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno >> > > Lossin suggested I also look into `clippy::ptr_cast_constness` and I >> > > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3 >> > > lints. It also enables `clippy::as_underscore` which ensures other >> > > pointer casts weren't missed. The first commit reduces the need for >> > > pointer casts and is shared with another series[1]. >> > > >> > > The final patch also enables pointer provenance lints and fixes >> > > violations. See that commit message for details. The build system >> > > portion of that commit is pretty messy but I couldn't find a better way >> > > to convincingly ensure that these lints were applied globally. >> > > Suggestions would be very welcome. >> > >> > I applied the patches to v6.14-rc7 and did a quick pass with > > So I rebased this on rust-next and fixed a few more instances (in > addition to enabling the extra lint), but I realized that rust-next is > still based on v6.14-rc5. I think we're going to have the same problem > here as in the &raw series; either Miguel is going to have to apply > fixups when picking these patches, or we have to split the fixes out > from the lints and land it over several cycles. Thoughts? One option is to pick the patches early in the cycle and then ask authors of patches to rebase. We did that with the alloc changes in the past. --- Cheers, Benno
On Mon, Mar 17, 2025 at 10:23 AM Tamir Duberstein <tamird@gmail.com> wrote: > > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno > Lossin suggested I also look into `clippy::ptr_cast_constness` and I > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3 > lints. It also enables `clippy::as_underscore` which ensures other > pointer casts weren't missed. The first commit reduces the need for > pointer casts and is shared with another series[1]. > > The final patch also enables pointer provenance lints and fixes > violations. See that commit message for details. The build system > portion of that commit is pretty messy but I couldn't find a better way > to convincingly ensure that these lints were applied globally. > Suggestions would be very welcome. > > Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1] > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > --- > Changes in v5: > - Use `pointer::addr` in OF. (Boqun Feng) > - Add documentation on stubs. (Benno Lossin) > - Mark stubs `#[inline]`. > - Pick up Alice's RB on a shared commit from > https://lore.kernel.org/all/Z9f-3Aj3_FWBZRrm@google.com/. > - Link to v4: https://lore.kernel.org/r/20250315-ptr-as-ptr-v4-0-b2d72c14dc26@gmail.com > > Changes in v4: > - Add missing SoB. (Benno Lossin) > - Use `without_provenance_mut` in alloc. (Boqun Feng) > - Limit strict provenance lints to the `kernel` crate to avoid complex > logic in the build system. This can be revisited on MSRV >= 1.84.0. > - Rebase on rust-next. > - Link to v3: https://lore.kernel.org/r/20250314-ptr-as-ptr-v3-0-e7ba61048f4a@gmail.com > > Changes in v3: > - Fixed clippy warning in rust/kernel/firmware.rs. (kernel test robot) > Link: https://lore.kernel.org/all/202503120332.YTCpFEvv-lkp@intel.com/ > - s/as u64/as bindings::phys_addr_t/g. (Benno Lossin) > - Use strict provenance APIs and enable lints. (Benno Lossin) > - Link to v2: https://lore.kernel.org/r/20250309-ptr-as-ptr-v2-0-25d60ad922b7@gmail.com > > Changes in v2: > - Fixed typo in first commit message. > - Added additional patches, converted to series. > - Link to v1: https://lore.kernel.org/r/20250307-ptr-as-ptr-v1-1-582d06514c98@gmail.com > > --- > Tamir Duberstein (6): > rust: retain pointer mut-ness in `container_of!` > rust: enable `clippy::ptr_as_ptr` lint > rust: enable `clippy::ptr_cast_constness` lint > rust: enable `clippy::as_ptr_cast_mut` lint > rust: enable `clippy::as_underscore` lint > rust: use strict provenance APIs > > Makefile | 4 ++ > init/Kconfig | 3 + > rust/bindings/lib.rs | 1 + > rust/kernel/alloc.rs | 2 +- > rust/kernel/alloc/allocator_test.rs | 2 +- > rust/kernel/alloc/kvec.rs | 4 +- > rust/kernel/block/mq/operations.rs | 2 +- > rust/kernel/block/mq/request.rs | 7 +- > rust/kernel/device.rs | 5 +- > rust/kernel/device_id.rs | 2 +- > rust/kernel/devres.rs | 19 +++--- > rust/kernel/error.rs | 2 +- > rust/kernel/firmware.rs | 3 +- > rust/kernel/fs/file.rs | 2 +- > rust/kernel/io.rs | 16 ++--- > rust/kernel/kunit.rs | 15 ++--- > rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++- > rust/kernel/list/impl_list_item_mod.rs | 2 +- > rust/kernel/miscdevice.rs | 2 +- > rust/kernel/of.rs | 6 +- > rust/kernel/pci.rs | 15 +++-- > rust/kernel/platform.rs | 6 +- > rust/kernel/print.rs | 11 ++-- > rust/kernel/rbtree.rs | 23 +++---- > rust/kernel/seq_file.rs | 3 +- > rust/kernel/str.rs | 18 ++---- > rust/kernel/sync/poll.rs | 2 +- > rust/kernel/uaccess.rs | 12 ++-- > rust/kernel/workqueue.rs | 12 ++-- > rust/uapi/lib.rs | 1 + > 30 files changed, 218 insertions(+), 97 deletions(-) > --- > base-commit: 498f7ee4773f22924f00630136da8575f38954e8 > change-id: 20250307-ptr-as-ptr-21b1867fc4d4 Per the discussion in today's Rust-for-Linux meeting and Benno's reply[0] please take this series without the last patch, whenever you see fit to do so. At this time no changes have been requested to the first 5 patches, so I am not planning to respin. Cheers. Tamir [0] https://lore.kernel.org/all/D8KIHNXCPE0P.K4MD7QJ1AC17@proton.me/
© 2016 - 2025 Red Hat, Inc.