| Message ID | 20250317-ptr-as-ptr-v5-0-5b5f21fa230a@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rust: reduce pointer casts, enable related lints | expand |
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/
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 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 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 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 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 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 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 :)
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 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 :) > >
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,