cmse: lint on unions crossing the secure boundary #147697
Conversation
folkertdev added F-cmse_nonsecure_entry 
rustbot added A-test-infra-minicore 
This comment has been minimized.
This comment has been minimized.
folkertdev force-pushed the cmse-lint-on-uninitialized branch from a344d30 to 514010e Compare This comment has been minimized.
This comment has been minimized.
folkertdev force-pushed the cmse-lint-on-uninitialized branch from 514010e to 9d276b5 Compare This comment has been minimized.
This comment has been minimized.
folkertdev force-pushed the cmse-lint-on-uninitialized branch from 9d276b5 to 4a269f5 Compare This comment has been minimized.
This comment has been minimized.
folkertdev force-pushed the cmse-lint-on-uninitialized branch from 4a269f5 to 0b424f6 Compare | r? @davidtwco This seems useful just for parity with clang. The code is built to be extended to cover more cases of types possibly containing uninitialized memory, but by the looks of things there isn't currently a straightforward way to detect such types (cc #t-compiler/help > check whether a type can be (partially) uninitialized) |
rustbot added S-waiting-on-review 
| #[no_mangle] | ||
| #[allow(improper_ctypes_definitions)] | ||
| pub extern "cmse-nonsecure-entry" fn f5(_: [u32; 5]) {} //~ ERROR [E0798] | ||
| // |
| use minicore::*; | ||
| | ||
| #[repr(Rust)] | ||
| pub union ReprRustUnionU64 { |
There was a problem hiding this comment.
Can you add cases where the unions are contained within other types to these tests?
davidtwco added the I-lang-nominated When a union passes from secure to non-secure (so, passed as an argument to an nonsecure call, or returned by a nonsecure entry), warn that there may be secure information lingering in the unused or uninitialized parts of a union value. https://godbolt.org/z/vq9xnrnEs
folkertdev force-pushed the cmse-lint-on-uninitialized branch from 0b424f6 to 4586300 Compare 
There was a problem hiding this comment.
The lint will only fire when the cmse ABIs are enabled, but I suppose the name does sort of "leak".
the OP here provides some context. The bigger picture is in this draft RFC that I plan to formally submit soon.
| warning: passing a union across the security boundary may leak information | ||
| --> $DIR/return-uninitialized.rs:46:5 | ||
| | | ||
| LL | / match 0 { | ||
| LL | | | ||
| LL | | 0 => Wrapper(ReprRustUnionU64 { _unused: 1 }), | ||
| LL | | _ => Wrapper(ReprRustUnionU64 { _unused: 2 }), | ||
| LL | | } | ||
| | |_____^ | ||
| | |
There was a problem hiding this comment.
would it make sense to warn in the individual arms of the match instead? I think as a user that would be better in this simple case, though I don't know that we can make that robust (e.g. thinking about labeled blocks).
4 participants
tracking issue: #81391
tracking issue: #75835
When a union passes from secure to non-secure (so, passed as an argument to a non-secure call, or returned by a non-secure entry), warn that there may be secure information lingering in the unused or uninitialized parts of a union value.
This lint matches the behavior of clang (see https://godbolt.org/z/vq9xnrnEs). Like clang we warn at the use site, so that individual uses could be annotated with
#[allow(cmse_uninitialized_leak)].Ideally we'd warn on any type that may have uninitialized parts, but I haven't figured out a good way to do that yet.
It is still unclear whether a union value where all fields are equally large and allow the same bit patterns can be considered initialized (see rust-lang/unsafe-code-guidelines#438), so for now we just warn on any
union.r? @ghost