rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.2k stars 12.7k forks source link

[strict provenance] add lints for evil pointer casts #95488

Closed Gankra closed 1 month ago

Gankra commented 2 years ago

This issue is part of the Strict Provenance Experiment - #95228

We should make it easier for people to detect places where they are using casts instead of the "blessed" strict_provenance APIs.

@eddyb and I prototyped this out here: https://github.com/rust-lang/rust/pull/95199/commits/93f7f06737686fdd6a44127d51129764c0d0a0bc

The patch needs some cleanups, though. Quoting from elsewhere:


All lints should be made allow by default, meaning they're opt-in.

At least in the bootstrap, the compiler will complain if you allow() a lint in your code that doesn't exist. This potentially just means:

Also due to the "Opaque Function Pointers" / "Harvard Architecture" / "AVR is cursed" issue

https://github.com/rust-lang/rust/blob/92804455704cc59d6d8272faf72f442c6125d395/library/core/src/ptr/mod.rs#L1390-L1395

I think we want the lint broken up into parts:

I can't justify discouraging fn <-> int, absent better ways to talk about fn ptrs properly.

jrtc27 commented 2 years ago
  • [oxford_casts] - casts that make harvard architectures sad -- fn<->ptr (name is a joke... unless...)

Von Neumann would be the appropriate name if you wanted to highlight that aspect, though some Harvard architectures can handle them just fine too, depends on whether the data pointer representation is sufficient to also safely round-trip function pointers (though it may refer to something else when interpreted as a data pointer).

beepster4096 commented 2 years ago

Can't we just make this lint unstable like unsafe_block_in_unsafe_fn used to be?

Gankra commented 2 years ago

Oh yes if there's properly lint stability stuff that would be Good To Use (unless that introduces "have to use nightly" and that is deemed unacceptable).

Gankra commented 2 years ago

Oh also I have no idea what the deal is with "safe transmute" stuff, but if these lints can at all look "into" transmutes and try to catch more Secret Casts that would be Friggin' Rad.

eternaleye commented 2 years ago
  • [oxford_casts] - casts that make harvard architectures sad -- fn<->ptr (name is a joke... unless...)

Von Neumann would be the appropriate name if you wanted to highlight that aspect, though some Harvard architectures can handle them just fine too, depends on whether the data pointer representation is sufficient to also safely round-trip function pointers (though it may refer to something else when interpreted as a data pointer).

I proposed this in side channels, with the (joking) explanation that "if you start with just a few, they'll proliferate" :P

jswrenn commented 2 years ago

Oh also I have no idea what the deal is with "safe transmute" stuff, but if these lints can at all look "into" transmutes and try to catch more Secret Casts that would be Friggin' Rad.

@Gankra Should be possible! The in-development safe(r) transmute API, which will be a distinct API from mem::transmute, is just going to flatly forbid pointer-to-integer and integer-to-pointer transmutations. It should be possible to use that same machinery to lint existing occurrences of mem::transmute.

Lokathor commented 2 years ago

Bytemuck also removed the Pod impl for pointers a few versions back <3

scottmcm commented 2 years ago

Note that lang and libs both want a lint for ptr-to-int via as (https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Adding.20methods.20as.20more.20specific.20versions.20of.20.60as.60/near/238391074) so if anyone wants to pick that up it should hopefully be uncontroversial.

niluxv commented 2 years ago

I would like to give it a try. @rustbot claim

Manishearth commented 2 years ago

@Gankra regarding "lints that don't exist", there's a lint deprecation mechanism that allows you to register names as deprecated (and delete the impl), it's used often in clippy and occasionally in rustc

niluxv commented 2 years ago

95599 contains lints for int-to-ptr and ptr-to-int as casts, and will likely be merged soon. @rustbot release-assignment

The exposing API methods introduced by #95588 might be linted against using clippy's disallowed_methods lint (although it doesn't seem possible yet to specify raw pointer methods).

Regarding transmutes it should be pretty trivial to lint against ptr-to-int and int-to-ptr transmutes, but not when the integers/pointers are hidden inside other structs. That will probably require the full safer transmute stuff.

RalfJung commented 1 month ago

The lints have their own dedicated feature and tracking issue now, so closing in favor of that: https://github.com/rust-lang/rust/issues/130351.