Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Other modules extern functions can currently be redefined #47449

Closed
gamozolabs opened this issue Jan 15, 2018 · 1 comment
Closed

Other modules extern functions can currently be redefined #47449

gamozolabs opened this issue Jan 15, 2018 · 1 comment
Labels
C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gamozolabs
Copy link

gamozolabs commented Jan 15, 2018

Currently it is possible to redefine functions that are defined as extern in another module. This affects many internal functions to rustc (the compiler) as well as the built Rust application.

The most bold issue is currently this is safe in stable, produces no warnings or errors, and causes a crash of the built program:

#[no_mangle]
pub fn memcpy() {}
pub fn main() {}

This crash can be observed directly on the Rust playground https://play.rust-lang.org/?gist=c914a0d307eff3b7d4bb2b90b264970e&version=stable

The issue at heart is being able to redefine extern functions from another module. For example a simple application like this is allowed:

pub mod imports {
    extern "C" {
        pub fn write();
    }
}

#[no_mangle]
pub fn write() {}

fn main() {}

Further this issue also affects rustc itself, in that it can invalidate certain internal compiler state and cause memory corruption and a crash of rustc.

// Build with 'rustc --crate-type lib --target x86_64-pc-windows-msvc src/lib.rs'
#[no_mangle]
pub static __CxxFrameHandler3: u32 = 0;

pub extern fn x() { panic!("WOO"); }

Building this will cause rustc to crash due to an out of bounds write due to type confusion around the type of __CxxFrameHandler3. This issue can cause some pretty catastrophic corruption resulting in rustc jumping to arbitrary code as seen here: https://gist.github.com/gamozolabs/d966adc2e13310a8ddb65285e5936681 (sorry, don't have symbols to this official build). Due to this being memory corruption, it might not always trigger a crash. Turning on page heap or address sanitizer makes it always crash, but without these it's 'random'.
Here's stack traces at each stage of this crash, and the call site of the allocation that is being written out of bounds to:
https://gist.github.com/gamozolabs/2fc773c092a37ce3b160e946b6d475a3

This would be an assert if LLVM assertions are turned on and it is related to #38641 .

Further this also can be caused on non-MSVC targets by doing it as such:

// Build with 'rustc --target x86_64-unknown-linux-gnu --crate-type lib src/lib.rs'
#![no_std]

#[no_mangle]
pub static rust_eh_personality: u32 = 0;

pub extern fn x() { panic!("WOO"); }

Further it seems LLVM errors can be caused by defining LLVM intrinsics such as:

#![crate_type = "lib"]

#[export_name = "llvm.lifetime.start"]
pub extern fn ABCD() {}

Output:

Attribute after last parameter!
void ()* @llvm.lifetime.start
LLVM ERROR: Broken function found, compilation aborted!

However if you try this with a made up LLVM intrinsic name, it yells at you for trying to define LLVM intrinsics:

#![crate_type = "lib"]

#[export_name = "llvm.lifetime.asdfasdf"]
pub extern fn ABCD() {}

Output:

llvm intrinsics cannot be defined!
void ()* @llvm.lifetime.asdfasdf
LLVM ERROR: Broken function found, compilation aborted!

At this point I've stopped looking for more examples of what this impacts as it's just too many to bother writing up. Almost all of my testing was done on x86_64 Windows as a host with nightly Rust, however I have cross checked most things on Linux or the Rust playground.

Further while almost all of these examples use #[export_name] or #[no_mangle], this issue also can occur if you have an extern that is named the same as your mangled Rust function. A bit niche, but this issue is not with just these attributes.

One worry with fixing this is that it would break a lot of #![no_std] projects that implement panic_fmt, memcpy, etc as they are defined as extern in Rust. I think to make this work we would have to enforce that the type signature of the extern matches the type signature of the function you are implementing. If you declare extern { pub unsafe extern "C" fn memcpy(...) } then the reimplementation must match that including the unsafe. Which at that point this is no longer unsound. Or it might be able to be made that any naming collisions with an extern {} must be unsafe. Haven't thought through all the edge cases of that.

-B

@pietroalbini pietroalbini added I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 23, 2018
@jonas-schievink
Copy link
Contributor

Looks like a duplicate of #28179, so closing in favor of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants