[folkertdev]

View all comments

tracking issue: #81391
tracking issue: #75835

Adds a cmse_uninitialized_leak lint.

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)].

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

[folkertdev]

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]

HIR ty lowering was modified

cc @fmease

This PR modifies tests/auxiliary/minicore.rs.

cc @jieyouxu

[davidtwco]

Implementation LGTM, two nits, but will need t-lang approval for a new lint

View changes since this review

[davidtwco]

Can you add cases where the unions are contained within other types to these tests?

[folkertdev]

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.

View changes since this review

[folkertdev]

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).

[joshtriplett]

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.

Would that also include padding?

In any case, this seems wildly useful for many users, not just cmse. The Linux kernel would likely benefit from this, for instance, to avoid leaking uninitialized bits to userspace.

On that basis I'm wondering if we should aspirationally call this uninitialized_leak or similar, without cmse in it.

Is there some means by which we could allow the user to suppress this not by allow but by demonstrating that they've properly zero-initialized it? Or is the point that LLVM doesn't guarantee that because it might un-zero that memory (e.g. copying without caring what value ends up in the padding / unused-union-bits / etc)?

[folkertdev]

Would that also include padding?

Ideally, yes. But I don't have a good strategy for actually achieving that. Based on #t-compiler/help > check whether a type can be (partially) uninitialized @ 💬 maybe there are parts of safe transmute that are helpful here.

Is there some means by which we could allow the user to suppress this not by allow but by demonstrating that they've properly zero-initialized it? Or is the point that LLVM doesn't guarantee that because it might un-zero that memory (e.g. copying without caring what value ends up in the padding / unused-union-bits / etc)?

I'm not familiar enough with the opsem details here, but in any case I don't think we'd want to tie our lints to LLVM implementation details that much?

[joshtriplett]

I'm not proposing to depend on LLVM details. I was asking how we can handle code that's doing the correct thing (whatever that might be), such as ensuring the uninitialized memory is zero.

In C, you would ensure the union is zero-initialized, then initialize the correct field, then return it. What's the equivalent operation that you can do in Rust, and can we ensure that we don't emit the lint if you properly do that?

[folkertdev]

What I had in mind is to use MaybeUninit::zeroed, set the relevant fields, and assume_init.

At least in that case, I don't think we have a good way of checking whether the value is properly initialized without actually evaluating the program (e.g. with miri).

[folkertdev]

@rustboy ready

@davidtwco can you give this another pass before I nominate it for T-lang. We discussed this lint during a recent design meeting (#t-lang/meetings > Design meeting 2026-07-22) so overall T-lang is on board, but I think lints need FCP regardless.

[davidtwco]

LGTM

View changes since this review

[folkertdev]

Nominating this for T-lang.

Something I'd like input on is the name, cmse_uninitialized_leak, and what it means.

It contains "cmse" because it triggers only on the cmse calling conventions. It is possible we want to generalize the behavior, so maybe the name should be more general.

The name does not contain "union". That is partially historical (earlier iterations of this lint also linted on enums), but also the lint is currently named after the concept that it is about, not the specifics of how it performs the check.

Because this is a lint, this PR will need T-lang FCP.

[Jules-Bertholet]

Because this is a lint, this PR will need T-lang FCP.

You can probably bundle this into the broader CMSE FCP?

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rust-bors]

☔ The latest upstream changes (presumably #160774) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

[traviscross]

Suggested change

	pub CMSE_UNINITIALIZED_LEAK,
	pub CMSE_UNINITIALIZED_LEAKS,

Following our RFC 0344 guidance, I'd suggest naming this in the plural.

View changes since the review

[traviscross]

Thanks @folkertdev. Sounds right. We talked about this in the lang triage meeting today. Let's propose to do this under the name cmse_uninitialized_leaks.

@rfcbot fcp merge lang

[tmandry]

@rfcbot reviewed

[nikomatsakis]

@rfcbot reviewed

[joshtriplett]

@rfcbot reviewed

[scottmcm]

IIRC it's now final that all unions might be uninit, so that note in the OP can be updated.

That does mean that this lint sounds like it could be quite noisy, but that's always fixable later. If people working on these enclaves say it's fine, then I don't feel compelled to block. It does make me wonder whether it would prefer to be a flow-sensitive thing on the definition where we could check in MIR whether the returned value is actually initialized or not. (Notably that would be critical to allow doing this on more-common types like enums.) But it's a lint, so that can happen later. And any future language feature to opt-in to a union(f32, u32) requiring full init could also be considered by this lint.

@rfcbot reviewed

[rust-rfcbot]

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔