Mixing crates using different panic modes may introduce UB if a non-Rust unwind "escapes" into panic=abort crate via "C-unwind" #96926
Assignees
Labels
C-bug
Category: This is a bug.
I-unsound
Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
P-high
High priority
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
BatmanAoD
changed the title
BatmanAoD
changed the title
BatmanAoD
changed the title
BatmanAoD
changed the title
|
There is a draft for a fix here: #86801 Edit: this would not quite fix the issue. Discussion here: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/soundness.20in.20mixed.20panic.20mode/near/281893151 |
RalfJung
added
the
I-unsound
rustbot
added
the
I-prioritize
Ah, that would indeed make it impossible to even trigger this in Miri, let alone detect it. Marking this as unsound for now, but I guess the alternative would be to say that if you are linking with external code that unwinds, it is your responsibility to ensure that all crates are compiled appropriately (and cargo will take care of that automatically). Do we even want to support mixed panics modes, given cargo doesn't and they can be unsound and are generally rather subtle? Could we have rsutc warn against or even error in case panic modes are mixed? |
RalfJung
changed the title
|
Also see the Zulip discussion. EDIT: oops that was already linked above. |
apiraino
added
the
T-compiler
|
@rustbot claim |
|
Assigning priority as discussed in the Zulip thread of the Prioritization Working Group. @rustbot label -I-prioritize +P-high |
rustbot
added
P-high
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
May 19, 2022
Remove libstd's calls to `C-unwind` foreign functions Remove all libstd and its dependencies' usage of `extern "C-unwind"`. This is a prerequiste of a WIP PR which will forbid libraries calling `extern "C-unwind"` functions to be compiled in `-Cpanic=unwind` and linked against `panic_abort` (this restriction is necessary to address soundness bug rust-lang#96926). Cargo will ensure all crates are compiled with the same `-Cpanic` but the std is only compiled `-Cpanic=unwind` but needs the ability to be linked into `-Cpanic=abort`. Currently there are two places where `C-unwind` is used in libstd: * `__rust_start_panic` is used for interfacing to the panic runtime. This could be `extern "Rust"` * `_{rdl,rg}_oom`: a shim `__rust_alloc_error_handler` will be generated by codegen to call into one of these; they can also be `extern "Rust"` (in fact, the generated shim is used as `extern "Rust"`, so I am not even sure why these are not, probably because they used to `extern "C"` and was changed to `extern "C-unwind"` when we allow alloc error hooks to unwind, but they really should just be using Rust ABI). For dependencies, there is only one `extern "C-unwind"` function call, in `unwind` crate. This can be expressed as a re-export. More dicussions can be seen in the Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/soundness.20in.20mixed.20panic.20mode `@rustbot` label: T-libs F-c_unwind
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this issue
Jun 28, 2022
Fix FFI-unwind unsoundness with mixed panic mode UB maybe introduced when an FFI exception happens in a `C-unwind` foreign function and it propagates through a crate compiled with `-C panic=unwind` into a crate compiled with `-C panic=abort` (rust-lang#96926). To prevent this unsoundness from happening, we will disallow a crate compiled with `-C panic=unwind` to be linked into `panic-abort` *if* it contains a call to `C-unwind` foreign function or function pointer. If no such call exists, then we continue to allow such mixed panic mode linking because it's sound (and stable). In fact we still need the ability to do mixed panic mode linking for std, because we only compile std once with `-C panic=unwind` and link it regardless panic strategy. For libraries that wish to remain compile-once-and-linkable-to-both-panic-runtimes, a `ffi_unwind_calls` lint is added (gated under `c_unwind` feature gate) to flag any FFI unwind calls that will cause the linkable panic runtime be restricted. In summary: ```rust #![warn(ffi_unwind_calls)] mod foo { #[no_mangle] pub extern "C-unwind" fn foo() {} } extern "C-unwind" { fn foo(); } fn main() { // Call to Rust function is fine regardless ABI. foo::foo(); // Call to foreign function, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. unsafe { foo(); } //~^ WARNING call to foreign function with FFI-unwind ABI let ptr: extern "C-unwind" fn() = foo::foo; // Call to function pointer, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. ptr(); //~^ WARNING call to function pointer with FFI-unwind ABI } ``` Fix rust-lang#96926 `@rustbot` label: T-compiler F-c_unwind
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
Jun 29, 2022
Fix FFI-unwind unsoundness with mixed panic mode UB maybe introduced when an FFI exception happens in a `C-unwind` foreign function and it propagates through a crate compiled with `-C panic=unwind` into a crate compiled with `-C panic=abort` (rust-lang#96926). To prevent this unsoundness from happening, we will disallow a crate compiled with `-C panic=unwind` to be linked into `panic-abort` *if* it contains a call to `C-unwind` foreign function or function pointer. If no such call exists, then we continue to allow such mixed panic mode linking because it's sound (and stable). In fact we still need the ability to do mixed panic mode linking for std, because we only compile std once with `-C panic=unwind` and link it regardless panic strategy. For libraries that wish to remain compile-once-and-linkable-to-both-panic-runtimes, a `ffi_unwind_calls` lint is added (gated under `c_unwind` feature gate) to flag any FFI unwind calls that will cause the linkable panic runtime be restricted. In summary: ```rust #![warn(ffi_unwind_calls)] mod foo { #[no_mangle] pub extern "C-unwind" fn foo() {} } extern "C-unwind" { fn foo(); } fn main() { // Call to Rust function is fine regardless ABI. foo::foo(); // Call to foreign function, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. unsafe { foo(); } //~^ WARNING call to foreign function with FFI-unwind ABI let ptr: extern "C-unwind" fn() = foo::foo; // Call to function pointer, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. ptr(); //~^ WARNING call to function pointer with FFI-unwind ABI } ``` Fix rust-lang#96926 `@rustbot` label: T-compiler F-c_unwind
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this issue
Jun 29, 2022
Fix FFI-unwind unsoundness with mixed panic mode UB maybe introduced when an FFI exception happens in a `C-unwind` foreign function and it propagates through a crate compiled with `-C panic=unwind` into a crate compiled with `-C panic=abort` (rust-lang#96926). To prevent this unsoundness from happening, we will disallow a crate compiled with `-C panic=unwind` to be linked into `panic-abort` *if* it contains a call to `C-unwind` foreign function or function pointer. If no such call exists, then we continue to allow such mixed panic mode linking because it's sound (and stable). In fact we still need the ability to do mixed panic mode linking for std, because we only compile std once with `-C panic=unwind` and link it regardless panic strategy. For libraries that wish to remain compile-once-and-linkable-to-both-panic-runtimes, a `ffi_unwind_calls` lint is added (gated under `c_unwind` feature gate) to flag any FFI unwind calls that will cause the linkable panic runtime be restricted. In summary: ```rust #![warn(ffi_unwind_calls)] mod foo { #[no_mangle] pub extern "C-unwind" fn foo() {} } extern "C-unwind" { fn foo(); } fn main() { // Call to Rust function is fine regardless ABI. foo::foo(); // Call to foreign function, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. unsafe { foo(); } //~^ WARNING call to foreign function with FFI-unwind ABI let ptr: extern "C-unwind" fn() = foo::foo; // Call to function pointer, will cause the crate to be unlinkable to panic-abort if compiled with `-Cpanic=unwind`. ptr(); //~^ WARNING call to function pointer with FFI-unwind ABI } ``` Fix rust-lang#96926 ``@rustbot`` label: T-compiler F-c_unwind
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is a case of undefined behavior around unwinding that will not be fixed by the stabilization of the
"C-unwind"ABI.Suppose there exists a C++ library with this function:
If this function is compiled as part of a crate with
panic=unwind:...and then called from a crate compiled with
panic=abort, the behavior is undefined, because the calling function will be compiled with the assumption that unwinding is impossible.Note:
cpp_unwindis declared withextern "C", the behavior will be well-defined: the runtime will abort even ifpanic=unwindis used.rustcdirectly (or via a different build system).panic!()implementation will be linked in the final binary, so usingpanic=abortwill preventpanic!()from unwinding the stack in the first place.The text was updated successfully, but these errors were encountered: