Enable kernel retpoline support

[emaste]

Summary

Security: CVE-2017-5715

[emaste]

sys/conf/kern.mk

221–223 ↗

(On Diff #39001)

Canonical use is .if defined(COMPILER_FEATURES) and if COMPILER_FEATURES is unset the feature is assumed to not exist. In this case I'd rather produce an error if the feature is requested but not available, and if COMPILER_FEATURES is unset for any reason just try to set -mretpoline. We should not silently fail to apply a requested security mitigation.

sys/conf/kern.opts.mk

91–95 ↗

(On Diff #39001)

I'm not sure of the best way to handle per-arch retpoline, presumably i386 support could arrive soon.

The technique may well be applicable to other architectures although other mitigations (i.e., inexpensive versions of x86 IBRS / IBPB) may be preferred and mean it remains an x86-specific technique.

[emaste]

Example of retpoline, from sys/cam/scsi/scsi_enc.c

Source:

static void
enc_status_updater(void *arg)
{
        enc_softc_t *enc;

        enc = arg;
        if (enc->enc_vec.poll_status != NULL)
                enc->enc_vec.poll_status(enc);
}

Generated asm:

enc_status_updater:
ffffffff80372b10:       55      pushq   %rbp
ffffffff80372b11:       48 89 e5        movq    %rsp, %rbp
ffffffff80372b14:       4c 8b 5f 60     movq    96(%rdi), %r11
ffffffff80372b18:       4d 85 db        testq   %r11, %r11
ffffffff80372b1b:       74 06   je      6 <enc_status_updater+0x13>
ffffffff80372b1d:       5d      popq    %rbp
ffffffff80372b1e:       e9 cd 08 00 00  jmp     2253 <__llvm_retpoline_r11>
ffffffff80372b23:       5d      popq    %rbp
ffffffff80372b24:       c3      retq
ffffffff80372b25:       66 66 2e 0f 1f 84 00 00 00 00 00        nopw    %cs:(%rax,%rax)

Retpoline implementation:

__llvm_retpoline_r11:
ffffffff803733f0:       e8 0b 00 00 00  callq   11 <__llvm_retpoline_r11+0x10>
ffffffff803733f5:       f3 90   pause
ffffffff803733f7:       0f ae e8        lfence
ffffffff803733fa:       eb f9   jmp     -7 <__llvm_retpoline_r11+0x5>
ffffffff803733fc:       0f 1f 40 00     nopl    (%rax)
ffffffff80373400:       4c 89 1c 24     movq    %r11, (%rsp)
ffffffff80373404:       c3      retq
ffffffff80373405:       cc      int3
ffffffff80373406:       cc      int3
ffffffff80373407:       cc      int3
ffffffff80373408:       cc      int3
ffffffff80373409:       cc      int3
ffffffff8037340a:       cc      int3
ffffffff8037340b:       cc      int3
ffffffff8037340c:       cc      int3
ffffffff8037340d:       cc      int3
ffffffff8037340e:       cc      int3
ffffffff8037340f:       cc      int3

Note that llvm thunks are going to change names to match the arbitrary and perhaps undesirable choices made by GCC; see https://reviews.llvm.org/rL324449.

[dim]

sys/conf/kern.mk

221 ↗

(On Diff #39001)

Ehm, should you not check ${COMPILER_FEATURES:Mretpoline} here? Or maybe both COMPILER_FEATURES *and* LINKER_FEATURES?

[emaste]

Use COMPILER_FEATURES not LINKER_FEATURES for testing compiler features as reported by @dim.

[emaste]

With this patch I see six register-indirect jmps/calls in a GENERIC kernel:

ffffffff80999047:       ff d0   callq   *%rax
ffffffff80e3c0d6:       ff d0   callq   *%rax
ffffffff80e5d5d0:       ff 14 24        callq   *(%rsp)
ffffffff80f3d79f:       ff 55 f8        callq   *-8(%rbp)
ffffffff80f88b40:       ff 54 24 10     callq   *16(%rsp)
ffffffff80f88b60:       ff 54 24 10     callq   *16(%rsp)

The corresponding symbols for these are: privcmd_ioctl, outofnmi, sigcode, hypercall_md, ia32_sigcode, freebsd4_ia32_sigcode.

There is one memory-indirect call/jmp:

alltraps_pushregs_no_rax:
...
ffffffff80e3b5fd:       ff 24 25 30 bf b0 81    jmpq    *-2119123152

[dim]

Maybe a general remark, will this MK_RETPOLINE option be separate from a similar option for userland? Also, does this take care of adding the required link flags when linking the kernel?

[emaste]

In D14242#298846, @dim wrote:

Maybe a general remark, will this MK_RETPOLINE option be separate from a similar option for userland? Also, does this take care of adding the required link flags when linking the kernel?

Would you suggest we rename this one MK_KERNEL_RETPOLINE and introduce a separate MK_USERLAND_RETPOLINE?
Linker flags are required only when linking dynamic executables, to generate a retpoline-style PLT.

[dim]

In D14242#298980, @emaste wrote:

In D14242#298846, @dim wrote:

Maybe a general remark, will this MK_RETPOLINE option be separate from a similar option for userland? Also, does this take care of adding the required link flags when linking the kernel?

Would you suggest we rename this one MK_KERNEL_RETPOLINE and introduce a separate MK_USERLAND_RETPOLINE?
Linker flags are required only when linking dynamic executables, to generate a retpoline-style PLT.

Well, if there is only one setting, users will have the choice to compile *everything* with, or without retpoline mitigation. Not sure if there is a scenario where one would want to have a mitigated kernel with a non-mitigated userland, or vice versa?

On the other hand, I now understand that the MK_ settings are normally not how kernel options are configured, they're more supposed to go into the config files like GENERIC, right?

[emaste]

In D14242#302122, @dim wrote:

Well, if there is only one setting, users will have the choice to compile *everything* with, or without retpoline mitigation. Not sure if there is a scenario where one would want to have a mitigated kernel with a non-mitigated userland, or vice versa?

Intel has a whitepaper at https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf that explains where retpoline fits in and where it's needed. We want a retpoline-enabled kernel to protect kernel secret data, while there may be cases in userland where after evaluation we decide it is not necessary to apply retpoline.

On the other hand, I now understand that the MK_ settings are normally not how kernel options are configured, they're more supposed to go into the config files like GENERIC, right?

This is one of the things I'm not completely sure of. We do have an existing case in MK_CTF - it is set in kernel configs via WITH_CTF=1. I could commit this change now and we could adjust it before MFC if necessary.

[imp]

In D14242#302964, @emaste wrote:

In D14242#302122, @dim wrote:

Well, if there is only one setting, users will have the choice to compile *everything* with, or without retpoline mitigation. Not sure if there is a scenario where one would want to have a mitigated kernel with a non-mitigated userland, or vice versa?

Intel has a whitepaper at https://software.intel.com/sites/default/files/managed/1d/46/Retpoline-A-Branch-Target-Injection-Mitigation.pdf that explains where retpoline fits in and where it's needed. We want a retpoline-enabled kernel to protect kernel secret data, while there may be cases in userland where after evaluation we decide it is not necessary to apply retpoline.

On the other hand, I now understand that the MK_ settings are normally not how kernel options are configured, they're more supposed to go into the config files like GENERIC, right?

This is one of the things I'm not completely sure of. We do have an existing case in MK_CTF - it is set in kernel configs via WITH_CTF=1. I could commit this change now and we could adjust it before MFC if necessary.

Users configure options with WITH_foo and WITHOUT_foo. Makefiles, and their kin, use test, and to a more limited extent set, MK_foo. The Makefile machinery has code to make sure that MK_foo is always set to some default value, and that users can override that behavior using WITH/WITHOUT. Since kernel config files set 'makeoptions' it should use WITH/WITHOUT.

[emaste]

In D14242#302967, @imp wrote:

Since kernel config files set 'makeoptions' it should use WITH/WITHOUT.

This is really what the question was about - should WITH_RETPOLINE be set in GENERIC, or via src.conf?

[imp]

In D14242#302968, @emaste wrote:

In D14242#302967, @imp wrote:

Since kernel config files set 'makeoptions' it should use WITH/WITHOUT.

This is really what the question was about - should WITH_RETPOLINE be set in GENERIC, or via src.conf?

I'd set it in GENERIC which makes it default. People can turn it off if they need to.

[jhb]

In D14242#302969, @imp wrote:

In D14242#302968, @emaste wrote:

In D14242#302967, @imp wrote:

Since kernel config files set 'makeoptions' it should use WITH/WITHOUT.

This is really what the question was about - should WITH_RETPOLINE be set in GENERIC, or via src.conf?

I'd set it in GENERIC which makes it default. People can turn it off if they need to.

Just to be clear, if you are using an external toolchain that doesn't support retpoline can you disable it from the command line or do you have to use a custom kernel config if it is in GENERIC? Should be easy to test today with 'pkg install amd64-xtoolchain-gcc' and 'make CROSS_TOOLCHAIN=amd64-gcc' with a patched source tree and updated GENERIC. Ideally you can use 'make WITHOUT_RETPOLINE=yes buildkernel' to override it, but it makes it a bit more of a PITA if you have to have a modified kernel config.

[imp]

In D14242#302984, @jhb wrote:

In D14242#302969, @imp wrote:

In D14242#302968, @emaste wrote:

In D14242#302967, @imp wrote:

Since kernel config files set 'makeoptions' it should use WITH/WITHOUT.

This is really what the question was about - should WITH_RETPOLINE be set in GENERIC, or via src.conf?

I'd set it in GENERIC which makes it default. People can turn it off if they need to.

Just to be clear, if you are using an external toolchain that doesn't support retpoline can you disable it from the command line or do you have to use a custom kernel config if it is in GENERIC? Should be easy to test today with 'pkg install amd64-xtoolchain-gcc' and 'make CROSS_TOOLCHAIN=amd64-gcc' with a patched source tree and updated GENERIC. Ideally you can use 'make WITHOUT_RETPOLINE=yes buildkernel' to override it, but it makes it a bit more of a PITA if you have to have a modified kernel config.

Options set on the command line should override options set in the Makefile, but I'm unsure if it actually does in this case. We will have to test it.

[emaste]

• rename KERNEL_RETPOLINE to separate it from possible future userland support
• default to no for now, with the intent of setting it in GENERIC as is done with CTF