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

#[link_section] is unsound on Harvard architectures #76507

Closed
H2CO3 opened this issue Sep 9, 2020 · 6 comments
Closed

#[link_section] is unsound on Harvard architectures #76507

H2CO3 opened this issue Sep 9, 2020 · 6 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@H2CO3
Copy link

H2CO3 commented Sep 9, 2020

As pointed out in this thread on URLO, it is possible to read arbitrary memory without unsafe using the link_section attribute on architectures with separate address spaces for code and data.

To cite OP's code:

// Store PROG_BLOB in program space, ".text" would also work
#[link_section = ".progmem"]
static PROG_BLOB: [u8; 128] = [42; 128];

fn main() -> ! {
    let mut serial = /* initialize a serial output */;

    let mut idx = 0;
    loop {
        // This access is illegal, because Rust will emit a normal load
        // instruction, whereas the data is in the program space,
        // requiring a special load instruction.
        let b = PROG_BLOB[idx];

        // Dumping arbitrary RAM data!
        ufmt::uwrite!(&mut serial, "{:?} ", b).void_unwrap();

        idx += 1;
        if idx == BIG_BLOB.len() {
            break
        }
    }
    loop {
        // Just loop forever
    }
}

I expected to see this happen: the code should not compile, as it reads OOB memory.

Instead, this happened: The code compiles and outputs incorrect values, indicating UB.

@H2CO3 H2CO3 added the C-bug Category: This is a bug. label Sep 9, 2020
@Nemo157
Copy link
Member

Nemo157 commented Sep 9, 2020

See also #28179 and #72209, maybe this attribute could also be pulled under the unsafe_code lint (even on non-Harvard architectures it seems plausible that there's some way to cause unsoundness with it depending on the default linker scripts?).

@jyn514 jyn514 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Sep 15, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 15, 2020
@jyn514 jyn514 added A-linkage Area: linking into static, shared libraries and binaries T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Sep 15, 2020
@apiraino
Copy link
Contributor

Assigning P-low as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@apiraino apiraino added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 16, 2020
@DemiMarie
Copy link
Contributor

I’m pretty sure #[link_section] is unsafe anyway. At a minimum I suspect you can segfault by putting a function in non-executable memory, or break assumptions by putting an (immutable) static with a nonzero value in a section that gets zeroed.

@jackh726 jackh726 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Oct 11, 2023
@geofft
Copy link
Contributor

geofft commented Aug 28, 2024

Now that we have #123757, the 2024 edition will require writing #[link_section] as #[unsafe(link_section)] and it is prohibited under #![forbid(unsafe_code)]. See this playground example. Does that resolve this issue? (There wasn't an expectation that we could get Rust to emit the right load operations, or something, right?)

@RalfJung
Copy link
Member

RalfJung commented Aug 28, 2024 via email

@RalfJung
Copy link
Member

Ah, but that's not an I-unsound issue, so maybe let's track adding documentation in #123757 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants