Safe Transmute: Require that source referent is smaller than destination#122438

jswrenn · 2024-03-13T13:41:12Z

BikeshedIntrinsicFrom currently models transmute-via-union; i.e., it attempts to provide a where bound for this function:

pub unsafe fn transmute_via_union<Src, Dst>(src: Src) -> Dst {
    use core::mem::*;

    #[repr(C)]
    union Transmute<T, U> {
        src: ManuallyDrop<T>,
        dst: ManuallyDrop<U>,
    }

    let transmute = Transmute { src: ManuallyDrop::new(src) };

    // SAFETY: The caller must guarantee that the transmutation is safe.
    let dst = transmute.dst;

    ManuallyDrop::into_inner(dst)
}

A quirk of this model is that it admits padding extensions in value-to-value transmutation: The destination type can be bigger than the source type, so long as the excess consists of uninitialized bytes. However, this isn't permissible for reference-to-reference transmutations (introduced in #110662) — extra referent bytes cannot come from thin air.

This PR patches our analysis for reference-to-reference transmutations to require that the destination referent is no larger than the source referent.

r? @compiler-errors

jswrenn · 2024-03-13T13:45:10Z

compiler/rustc_transmute/src/maybe_transmutable/mod.rs

+                                        } else if dst_ref.size() > src_ref.size() {
+                                            Answer::No(Reason::DstRefIsTooBig {
+                                                src: src_ref,
+                                                dst: src_ref,
+                                            })


(This is the critical change of the PR.)

compiler-errors · 2024-03-13T14:59:29Z

compiler/rustc_transmute/src/layout/mod.rs


+    impl<'tcx> fmt::Display for Ref<'tcx> {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.write_char('&')?;


Not printing the region here too?

We could, but it's not relevant to the error message this Display routine is currently used in, and I don't anticipate it will be relevant to other transmute error messages, since regions do not impact layout.

compiler-errors · 2024-03-13T15:01:47Z

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

+                        let src_size = src.size;
+                        let dst_size = dst.size;
+                        format!(
+                            "The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes"


Suggested change

"The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes"

"The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes)"

compiler-errors · 2024-03-13T15:13:55Z

tests/ui/transmutability/references/unit-to-u8.stderr

   |
 LL |     assert::is_maybe_transmutable::<&'static Unit, &'static u8>();
-   |                                                    ^^^^^^^^^^^ The size of `Unit` is smaller than the size of `u8`
+   |                                                    ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&Unit` (0) bytes


This still doesn't look right 😆

Suggested change

| ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&Unit` (0) bytes

| ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&Unit` (0 bytes)

Oof, I need to sleep more.

Done?

compiler-errors

Please triple check the error messages.

compiler-errors · 2024-03-13T15:23:52Z

compiler/rustc_transmute/src/maybe_transmutable/mod.rs

+                                        } else if dst_ref.size() > src_ref.size() {
+                                            Answer::No(Reason::DstRefIsTooBig {
+                                                src: src_ref,
+                                                dst: src_ref,


Suggested change

dst: src_ref,

dst: dst_ref,

The source referent absolutely must be smaller than the destination referent of a ref-to-ref transmute; the excess bytes referenced cannot arise from thin air, even if those bytes are uninitialized.

jswrenn · 2024-03-13T15:57:02Z

tests/ui/transmutability/references/reject_extension.stderr

+error[E0277]: `&Packed<Two>` cannot be safely transmuted into `&Packed<Four>`
+  --> $DIR/reject_extension.rs:48:37
+   |
+LL |     assert::is_transmutable::<&Src, &Dst>();


I don't like that only the second parameter is ^^^^, but that's a general issue with all of our error messages that I'll file a follow-up PR for.

Otherwise, this error message LGTM.

compiler-errors · 2024-03-13T17:18:32Z

@bors r+ rollup

bors · 2024-03-13T17:18:35Z

📌 Commit 216df4a has been approved by compiler-errors

It is now in the queue for this repository.

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#122422 (compiletest: Allow `only-unix` in test headers) - rust-lang#122424 (fix: typos) - rust-lang#122425 (Increase timeout for new bors bot) - rust-lang#122426 (Fix StableMIR `WrappingRange::is_full` computation) - rust-lang#122429 (Add Exploit Mitigations PG to triagebot.toml) - rust-lang#122430 (Generate link to `Local` in `hir::Let` documentation) - rust-lang#122434 (pattern analysis: rename a few types) - rust-lang#122437 (pattern analysis: remove `MaybeInfiniteInt::JustAfterMax`) - rust-lang#122438 (Safe Transmute: Require that source referent is smaller than destination) - rust-lang#122442 (extend docs of -Zprint-mono-items) - rust-lang#122449 (Delay a bug for stranded opaques) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#122438 - jswrenn:check-referent-size, r=compiler-errors Safe Transmute: Require that source referent is smaller than destination `BikeshedIntrinsicFrom` currently models transmute-via-union; i.e., it attempts to provide a `where` bound for this function: ```rust pub unsafe fn transmute_via_union<Src, Dst>(src: Src) -> Dst { use core::mem::*; #[repr(C)] union Transmute<T, U> { src: ManuallyDrop<T>, dst: ManuallyDrop<U>, } let transmute = Transmute { src: ManuallyDrop::new(src) }; // SAFETY: The caller must guarantee that the transmutation is safe. let dst = transmute.dst; ManuallyDrop::into_inner(dst) } ``` A quirk of this model is that it admits padding extensions in value-to-value transmutation: The destination type can be bigger than the source type, so long as the excess consists of uninitialized bytes. However, this isn't permissible for reference-to-reference transmutations (introduced in rust-lang#110662) — extra referent bytes cannot come from thin air. This PR patches our analysis for reference-to-reference transmutations to require that the destination referent is no larger than the source referent. r? `@compiler-errors`

rustbot assigned compiler-errors Mar 13, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 13, 2024

jswrenn commented Mar 13, 2024

View reviewed changes

compiler-errors suggested changes Mar 13, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2024

jswrenn force-pushed the check-referent-size branch from b949a37 to 107a473 Compare March 13, 2024 15:11

compiler-errors suggested changes Mar 13, 2024

View reviewed changes

jswrenn force-pushed the check-referent-size branch from 107a473 to 422fb2e Compare March 13, 2024 15:17

compiler-errors suggested changes Mar 13, 2024

View reviewed changes

safe transmute: require that src referent is smaller than dst

216df4a

The source referent absolutely must be smaller than the destination referent of a ref-to-ref transmute; the excess bytes referenced cannot arise from thin air, even if those bytes are uninitialized.

jswrenn force-pushed the check-referent-size branch from 422fb2e to 216df4a Compare March 13, 2024 15:54

jswrenn commented Mar 13, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 13, 2024

matthiaskrgr mentioned this pull request Mar 13, 2024

Rollup of 11 pull requests #122454

Merged

bors merged commit 89c3fa9 into rust-lang:master Mar 14, 2024

rustbot added this to the 1.78.0 milestone Mar 14, 2024

	"The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes"
	"The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes)"

	\| ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&Unit` (0) bytes
	\| ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&Unit` (0 bytes)

Uh oh!

Comments

Conversation

jswrenn commented Mar 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Mar 13, 2024

Uh oh!

bors commented Mar 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants