Tracking issue for the x86-interrupt calling convention

[phil-opp]

Overview

Tracking issue for the x86-interrupt calling convention, which was added in PR #39832. The feature gate name is abi_x86_interrupt. This feature will not be considered for stabilization without an RFC.

The x86-interrupt calling convention can be used for defining interrupt handlers on 32-bit and 64-bit x86 targets. The compiler then uses iret instead of ret for returning and ensures that all registers are restored to their original values.

Usage

extern "x86-interrupt" fn handler(stack_frame: ExceptionStackFrame) {…}

for interrupts and exceptions without error code and

extern "x86-interrupt" fn handler_with_err_code(stack_frame: ExceptionStackFrame,
                                                error_code: u64) {…}

for exceptions that push an error code (e.g., page faults or general protection faults). The programmer must ensure that the correct version is used for each interrupt.

For more details see the LLVM PR, and the corresponding proposal.

Known issues

• An earlier version of this description stated that the ExceptionStackFrame is passed by reference (instead of by value). This used to work on older LLVM version, but no longer works on LLVM 12. See Tracking issue for the x86-interrupt calling convention #40180 (comment) for more details.
• LLVM doesn't preserve MMX and x87 floating point registers across interrupts. This issue was reported in __attribute__((interrupt)) trashes MMX registers llvm/llvm-project#26785.
• (fixed) x86-interrupt calling convention leads to wrong error code in debug mode (x86-interrupt calling convention leads to wrong error code in debug mode #57270)

64-bit

• The x86_64 automatically aligns the stack on a 16-byte boundary when an interrupts occurs in 64-bit mode. However, the CPU pushes an 8-byte error code for some exceptions, which destroys the 16-byte alignment. At the moment, LLVM doesn't handle this case correctly and always assumes a 16-byte alignment. This leads to alignment issues on targets with SSE support, since LLVM uses misaligned movaps instructions for saving the xmm registers. This issue is tracked as bug 26413.

  A fix for this problem was submitted in D30049 and merged in rL299383.

• LLVM always tries to backup the xmm registers on 64-bit platforms even if the target doesn't support SSE. This leads to invalid opcode exceptions whenever an interrupt handler is invoked.

  The fix was merged to LLVM trunk in rL295347. Backported in x86 interrupt calling convention: only save xmm registers if the target supports SSE llvm#63.

• __attribute__((interrupt)) Handlers Dangerously Out of Spec on x86-64 llvm/llvm-project#41189

32-bit

• In 32-bit mode, the CPU performs no stack alignment on interrupts. Thus, the interrupt handler should perform a dynamic stack alignment (i.e. and esp, 16). However, LLVM doesn't do that at the moment, which might lead to alignment errors, especially for targets with SSE support. This issue is tracked in interrupt handler calls functions with misaligned stack llvm/llvm-project#26851.

[rexlunae]

If we're going to have an x86-interrupt abi, would it also make sense to have an x86-syscall? Or x86-sysenter?

[kyrias]

(D30049 was merged to LLVM trunk on the 3rd of April, for the record.)

[phil-opp]

@kyrias Thanks for the hint, I updated the issue text. I'll try to create a backport PR in the next few days.

[steveklabnik]

Triage: not aware of any movement stabilizing this.

Personal note: this is one of my favorite rust features :)

[kyrias]

Both of the 64-bit issues have long since been fixed, and Rust's minimum LLVM is newer than the fix, so it's just the MMX/x87 floating point and 32-bit issues remaining, which appear to have been untouched since 2017.

[phil-opp]

@kyrias I updated the issue description. I also added #57270, which will be hopefully fixed soon with the LLVM 9 upgrade.

[josephlr]

@phil-opp I think this issue can be updated to cross off #57270

On the x87/MMX issue, we should just mandate that to use the interrupt ABI, you have to build the code with target-features of -x87 and -mmx (-mmx could be a default now that MMX support is gone from Rust). It looks like LLVM and GCC just never intend to support saving/restoring the MMX/x87 registers, so it's probably not worth trying to make it work (as nobody should be using those features anyway.

We may also want to similarly disallow all SSE code in interrupt handlers (GCC bans SSE code, Clang allows it), as that might make it easier to make sure we are doing the right thing.

[phil-opp]

@phil-opp I think this issue can be updated to cross off #57270

Done!

[mark-i-m]

Just came across the following from a conference paper, and thought I would quote here as a data point:

We implement the low-level interrupt entry and exit code in assembly. While Rust provides support for the x86-interrupt function ABI (a way to write a Rust function that takes the x86 interrupt stack frame as an argument), in practice, it is not useful as we need the ability to interpose on the entry and exit from the interrupt, for example, to save all CPU registers.

(section 4.1 of https://www.usenix.org/system/files/osdi20-narayanan_vikram.pdf)

[repnop]

After discussing this on the Rust Community Server #os-dev channel with some folks (cc @Soveu @asquared31415), and checking out the LLVM patch that includes the ABI, it seems like taking the struct by-reference is incorrect and leads to undefined behavior (as seen in rust-osdev/x86_64#240). This seems to have worked previously, but I suspect the upgrade to LLVM 12 may have "fixed" the behavior. (edit: asquared tested both 2021-03-03 and 2021-03-05 nightly releases, both by-value and by-ref work on the former, but by-ref breaks on the latter, which I think pretty much confirms the LLVM 12 upgrade fixed the behavior)

The patch to LLVM includes the following code, suggesting that the very first parameter should always be by-value and never by-reference.

if (F.getCallingConv() == CallingConv::X86_INTR) {
    // IA Interrupt passes frame (1st parameter) by value in the stack.
    if (Idx == 1)
        Flags.setByVal();
}

[Soveu]

By the way, if the function signature contains more than 2 arguments LLVM crashes with

LLVM ERROR: unsupported x86 interrupt prototype

[phil-opp]

@repnop Thanks a lot for investigating this! I'll look into it and update the description of this issue if I can confirm that it works with a by-value parameter. Given that the Rust implementation just sets the LLVM calling convention, this shouldn't require any changes on the Rust side to fix it. We should adjust the x86_64 crate though.

To prevent issues like this (and wrong number of arguments errors), we should probably introduce some kind of lint that checks the function signature when the x86-interrupt calling convention is used. It's probably difficult to check the layout of the ExceptionStackFrame struct in detail, but verifying the number of parameters and the correct argument types (struct and optional u64) should be possible. I don't have any experience with creating lints though, so I'm not sure about the details.

[Soveu]

Also, we have found with @asquared31415 that using the second argument can cause stack corruption, because LLVM does not check if there is an error code and just pops unconditionally

[Andy-Python-Programmer]

I'll look into it and update the description of this issue if I can confirm that it works with a by-value parameter. Given that the Rust implementation just sets the LLVM calling convention, this shouldn't require any changes on the Rust side to fix it. We should adjust the x86_64 crate though.

It does work by-value parameter!

Everything now seems to be correct. Eg the stack segment is (16) => 0x10 which is valid

Thanks @repnop and @phil-opp!

[Andy-Python-Programmer]

Discovered something more:
As we are writing to 0xdeadbeaf the stack segment should be 0x00. For example this is a right interrupt stack:

(Picture from OS Phillip's Blog)
As you can see the image below its 0x10.

[Soveu]

Shouldn't 0x0 segment be null descriptor? (which iirc is invalid, stack segment should be a 64bit data segment)

[Soveu]

This is expected because there is unfortunately no good way to find out if there is an error code on the stack

My hack around this:

• Add two fields to InterruptInfo: error_code: u64 and has_error_code: u64
• When calling, check for alignment and either push 1 to set has_error_code or push 0 two times to zero error_code and clean has_error_code
• Then either jump directly into extern "x86-interrupt" code or save registers and then jump into code with different calling convention

(I haven't tested it though)

            test sp, 15
            jz no_error_code

            push 1
            jmp continue_to_handler

        no_error_code:
            push 0
            push 0

        continue_to_handler:
        // remember about `cld` as direction flag could be set

[asquared31415]

One potential problem I noticed is that modifying the interrupt stack frame does no longer work reliably with the by-value parameter. It works in debug mode, but not in release mode.

Using ptr::write_volatile does work to modify the interrupt stack. It's not a great way to do it though. Ideally code like

pub extern "x86-interrupt" fn handler(mut stack: InterruptStackFrame) {
    stack.ip = 0;
}

would work, however the compiler currently optimizes that write out. It would be best if the compiler could realize that modifying the interrupt stack frame does have an effect through the effect of iretq.
In the meantime this code works as expected to modify the stack frame

pub extern "x86-interrupt" fn handler(mut stack: InterruptStackFrame) {
    unsafe {
        addr_of_mut!(stack.ip).write_volatile(0x0);
    }
}

[Soveu]

On the other hand, it is just consistent with other calling conventions, where changing preserved registers or return pointer is UB

[phil-opp]

@asquared31415 Good idea! I updated my x86_64 PR to use a volatile wrapper (and added a note about the potential unsafety).

[asquared31415]

While I agree that changing those things is normally UB, certain interrupt handlers, especially the debug interrupts, are expected to set some values in the pushed RFLAGS register (normally the resume flag) so that the state is right when it returns. This is the only case where changing saved state is expected that I can think of, so I don't know if there's precedent as to how to handle this.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting.

We agree that this needs an RFC. Not specifically for x86-interrupt, but an "target-specific interrupt calling conventions" RFC. Once that RFC goes in, new target-specific interrupt calling conventions would just need a patch to the ABI section of the reference, ideally documenting the calling convention.

Would someone involved in this thread be up for writing that RFC?

@phil-opp Would you be willing to write that RFC?

We'd also appreciate an update to the API evolution RFC, to note that target-specific calling conventions are only as stable as the targets they're supported on; if we remove a target (for which we have guidelines and process in the target tier policy), removing its associated target-specific calling conventions is not a breaking change.

[josephlr]

I think this is a problem that needs to be solved before x86-interrupt can be stabilized. To give an idea of the severity of this issue, let's say a user-space program executes the instruction int 14 (which invokes the page fault exception handler). In this case no error code is pushed since it's just a software interrupt, not a real page fault. The problem is that the page fault exception handler is still expecting an error code, which isn't there, resulting in stack corruption, with everything off by 8 bytes.

The problem described here is exactly why you must have DPL == 0 in any IDT entry for an exception (i.e. vectors 0 though 31). That way, a usermode process cannot corrupt the kernel stack or (more generally) trigger the kernel's interrupt handling code in unexpected ways. With this setup, if usermode executes int 14, you just get a #GP.

As it stands currently, having an IDT entry directly reference an x86-interrupt function that takes an error code is unsound. You would instead need to use a trampoline function to work around the issue as in @Soveu's solution above.

I don't think this is true. It's the calling code's responsibility to make sure that each interrupt handler invocation matches its function signature, including:

• If there is an error code pushed or not
• The InterruptStackFrame having the correct size/alignment

While the calling convention doesn't provide these protections itself (as its just abound declaring functions), things like the x86_64 crate can help here. It's also worth noting that the "check sp alignment and conditionally push values" trampoline code linked above is not what Linux does. Based on the IDT vector, you should always know if an error code is present or not.

In short, I don't think any of the above issues should block stabilization. Of course, the LLVM crashes, error messages, and known issues probably need to be addressed before stabilization.

[phil-opp]

@joshtriplett Yes, I'm happy to work on this! I will create a rough draft and then open a pre-rfc thread on internals to discuss the details.

[mikeleany]

I think this is a problem that needs to be solved before x86-interrupt can be stabilized. To give an idea of the severity of this issue, let's say a user-space program executes the instruction int 14 (which invokes the page fault exception handler). In this case no error code is pushed since it's just a software interrupt, not a real page fault. The problem is that the page fault exception handler is still expecting an error code, which isn't there, resulting in stack corruption, with everything off by 8 bytes.

The problem described here is exactly why you must have DPL == 0 in any IDT entry for an exception (i.e. vectors 0 though 31). That way, a usermode process cannot corrupt the kernel stack or (more generally) trigger the kernel's interrupt handling code in unexpected ways. With this setup, if usermode executes int 14, you just get a #GP.

Thank you for the correction. DPL == 0 does indeed prevent the issue of user-level code corrupting the kernel stack in this way.

In short, I don't think any of the above issues should block stabilization. Of course, the LLVM crashes, error messages, and known issues probably need to be addressed before stabilization.

Given that my concern about user-level code corrupting the kernel stack was incorrect, I agree.

[josephlr]

Should we change the bug references above to point to:

• interrupt handler calls functions with misaligned stack llvm/llvm-project#26851
• __attribute__((interrupt)) Handlers Dangerously Out of Spec on x86-64 llvm/llvm-project#41189
• __attribute__((interrupt)) trashes MMX registers llvm/llvm-project#26785

As the LLVM project has moved to GitHub Issues.

[phil-opp]

Should we change the bug references above to point to:

Thanks for the suggestion! I updated/added the references you mentioned.

[phil-opp]

I created a Pre-RFC for initial feedback at https://internals.rust-lang.org/t/pre-rfc-interrupt-calling-conventions/16182. Please let me know what you think!

[chorman0773]

Note: This is unsound on 32-bit x86 (Protected mode) if llvm expects the esp/ss field.

When the processor is in legacy mode, esp and ss are pushed by the interrupt (and popped by iret) only if the interrupt crosses privilege levels (gate.RPL<CPL for interrupt, retcs.RPL>CPL for iret)