IssueTracker

Could you CC the reviewers to this bug so that they can access it - just so we can get the relevant people involved.

Also is it possible to attach the patch to this bug report so we can see what is involved on the clang side?

adding Chandler, who will have useful opinions on landing mitigations for embargoed vulnerabilities in the LLVM tree

In the past (e.g. when Intel disclosed LVI) our approach was to have patches ready to go and pre-reviewed by appropriate (and appropriately read-in) code owners but not post the patches "for real" until the embargo lifted. Usually this was part of a larger comms strategy where announcements were made in other forums.

Is there an entity beyond security@kernel.org that's coordinating this disclosure?

(re-reading, my second paragraph just repeats what you already laid out. Good independent verification but apologies for not reading the dates more carefully.)

I don't have permission to CC folks it seems, but it would be worth adding:

* Aaron Ballman <aaron@aaronballman.com> (a code owner for clang who has already reviewed my patch off list)
* Craig Topper <craig.topper@gmail.com> (a code owner for the x86 backend who has already reviewed my patch off list)

x86 is the only confirmed architecture affected. ARM has confirmed they're not affected for their micro-architectures. Not all x86 micro-architectures are affected.  Not seeing one particular x86 vendor represented in the LLVM Security Group is...concerning.

> Also is it possible to attach the patch to this bug report so we can see what is involved on the clang side?

Yes, but just note that the LLVM code is somewhat a giveaway of what's going on here.  GCC has had a similar feature for years; they did not publish the patch under embargo but rather did so publicly back when various spectre and meltdown mitigations were being tested before the development of retpoline.  The very first comment on the GCC mailing list was along the lines of "What is this?"  I'd like to avoid that here if possible; the name of the game is to not spill the beans to too wide an audience prematurely.  Doing so would lose LLVM their (lone) seat at the table for Linux kernel vulnerability disclosures such as this.  So we need to be super careful about keeping this need to know.

> Is there an entity beyond security@kernel.org that's coordinating this disclosure?

The encrypted kernel mailing list I'm referring to that is coordinating is not that specific email address.  It's more controlled than even that list and members of that list (security@kernel.org) aren't even considered need to know for this one. I don't know whether linux-distros@vs.openwall.org has even been contacted yet; the kernel patch set is up to ~45 patches and growing, and backports will be painful.

I'm curious what you want from us, if the vulnerability is too sensitive to talk about with (or show a mitigation patch to) the people who have signed up to be responsible about security vulnerabilities.

[Empty comment from Monorail migration]

It seems like the question is: what is the best way for Nick to prepare for this patch to be available in LLVM, so it can benefit LLVM users as soon as possible after the vulnerability is disclosed?

If that's the question, I think the answer is already reached in the discussion above: to the extent possible, get the patch reviewed early by code owners and resolve any concerns there; then post the patch to Phabricator when the embargo lifts. (Absent some tricky KAISER-style subterfuge, posting the patch should be considered equivalent to a public disclosure of the vulnerability.)

Open to others on the list refining or rebutting that message, but otherwise -- Nick, is there anything else you think you need from us?

> I'm curious what you want from us
> Nick, is there anything else you think you need from us?

Maybe if Aaron and Craig can confirm here that they've reviewed the patch, and if they'll be available when the embargo is scheduled to lift to publicly Accept the patch?

Otherwise, is there anything else I should be doing as part of the process?

> posting the patch should be considered equivalent to a public disclosure of the vulnerability.

Correct.  I don't mind posting it here, but it MUST NOT be posted to phab until the embargo lift (scheduled for Tuesday July 12 2022 9am PDT).  I'm more than happy to spend the rest of the week iterating on feedback of the initial design, but we need to ship a mitigation ASAP in a few open source repositories.  The code has been reviewed by trusted reviewers and tested by trusted kernel developers, so I'm not looking to re-architect the patch one week out from embargo lift.

Patch attached.

Couple comments on the patch

"be replace with" -> "be replaced with" in the LangRef.rst change

The include list in X86ReturnThunks.cpp seems to have more files than needed. StringSwitch.h was an obvious extra.

nit:    "Modified |= true;"    it's pretty uncommon for "|= true" to appear in the tree. Nearly everywhere does Modified = true.

I obviously don't know the exact details of the exploit but is replacing ret with a jmp absolutely necessary - my experience in other domains suggests that messing with the x86 call stack cache does pretty terrible things to performance.
If it is, do we know if this is actually x86 specific? zero-daying every other architecture is also not a super great outcome.

@oliver: my understanding of the patch is that it is not messing up with the call stack, just using a trampoline. So I assume it would be nice to have the trampoline target in a hot code section as it's likely to be shared by a lot of functions, but that's it, right?

My specific experience was JIT and interpreter insanity on x86_64, where there was a measurable impact because x86 (at least intel) include a call stack predictor and we did have a measurable cost from do some messing with the return address of functions.

To be clear: I am not saying this is a show stopper or anything like that, we were benchmarking JS so the type of code that is running in such is obviously different from pretty much anything else, this is more that in my experience this kind of manipulation *can* have 2nd order(?) perf impact, so I'd like to know someone has checked to make sure nothing unusable happens.

Pessimistic me would have a benchmark that was measuring deep recursion and iterative calling of an empty function. Not because it's representative, but just to get what is the impact in the worst possible case we can come up with. If it's not significant in that case there's no reason to spend any time doing the more complicated task of seeing if it impacts anything in real world code.

If there is a non-negligible impact we may be able to change the exact code gen or similar to compensate.

Or we may just accept the cost as being outweighed by the impact (as happened with SPECTRE, etc)

> Maybe if Aaron and Craig can confirm here that they've reviewed the patch, and if they'll be available when the embargo is scheduled to lift to publicly Accept the patch?

I can't speak for Craig, but I reviewed the Clang frontend bits and am happy with them. I will be around on Jul 12 and should have no trouble accepting the patch pretty quickly.

Re https://crbug.com/llvm/33#c12, yes, I think it's very safe to say (with no special knowledge about this particular vulnerability) that the impact on performance was evaluated and judged to be worth it given the security implications. As you point out, similar tradeoffs have been made in mitigating other transient execution vulnerabilities.

Aaron, thanks for the confirmation in https://crbug.com/llvm/33#c13 that you're ready to review+approve when the embargo lifts. I think having those commitments here is the best outcome for this bug.

I have reviewed the backend portion and you can see some comments in https://crbug.com/llvm/33#c9. Those comments were very minor. I'm happy with the backend portion. I will be around on Jul 12 and can approve quickly as well.

Thanks Craig!

Righto

Updated with Craig's latest comments.

> is replacing ret with a jmp absolutely necessary

I think so; it might ultimately not be, but it is how Linux kernel developers have decided to mitigate the vulnerability for the time being. It's not my call though, I was just asked to implement -mfunction-return=/__attribute__((function_return(""))) like GCC has already had now for years.  In order for clang to be useful as a drop in replacement for GCC, clang needs to match command line flag, function attribute, and codegen ABI frequently.

> my experience in other domains suggests that messing with the x86 call stack cache does pretty terrible things to performance.

Yep, the performance overhead of this isn't great for sure.  An additional set of kernel patches are in the works to try to claw back some performance lost to this approach, but they're not pretty and haven't been vetted quite yet.  Time will tell if this is the ultimate solution to this issue, but for now it's what we've got.

> If it is, do we know if this is actually x86 specific?

Not all x86 uArchs are effected.  The exploit is specific to uArch's the speculate return addresses in a specific way, and how they behave under specific circumstances wrt. kernel vs userspace privilege boundaries.  But there are at least two x86 vendors effected.

ARM commented that their micro architectures were not affected.  I'm guessing that they can't vouch for their architectural licensees micro architectures though.  That said, I can ask on the encrypted list about architectural licensees, and whether the reporters have tested other architectures or uArchs or contacted other vendors.

> zero-daying every other architecture is also not a super great outcome.

I agree but again it's not my call and there's no stopping the train now; regardless of my toolchain patch the embargo will lift July 12.  The researchers that reported the vulnerability may not have access to every uArch under the sun, and I don't know who they have or have not contacted about this vulnerability.

That said, there are non-Linux-kernel vendors participating in the encrypted list IIUC.  It might be worthwhile for security representatives at various organizations to get involved.
https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html

There's also other non-Linux operating system vendors on the linux-distros mailing list, which also has early access to embargo'd vuln reports+fixes:
https://oss-security.openwall.org/wiki/mailing-lists/distros#how-to-use-the-lists

FLOSS projects have a catch-22 of needing to develop mitigations behind closed doors, without publishing mitigations before embargo lift (similar to what we're doing here but for the toolchain). Those are the methods open source operating system developers have converged on, and made available to folks developing proprietary operating systems to participate in.

Nick, for us distribution packager to prepare, can you give us an hint whether this patch impacts BPF, which (if I'm not mistaken) always requires LLVM, even for GCC-built Linux?

From my understanding of the Linux kernel mitigation patches, which I have access to but cannot share outside the encrypted mailing list (much to the dismay of my employer), BPF programs are affected BUT it's the BPF JIT's x86 backend within the Linux kernel which is affected, not the BPF interpreter or the programs themselves. So I don't expect that BPF programs will need to be recompiled; instead distributions may need to either ship kernels with up to date mitigations, or consider disabling the BPF JIT.

That said, it may be better to ask on the linux-distros list once the kernel mitigations are published there (I don't know if they are, please don't spill the beans otherwise!).  Once the patches are public, I can point to any formal recommendations they may have.

But right now, it looks like Patch 10/45 addresses the x86 BPF JIT in the corresponding (and unpublished) Linux kernel patches.

My toolchain patch is orthogonal to the x86 BPF JIT.  BPF programs aren't expected to be compiled with -mfunction-return, use __attribute__((function_return(""))), or rely on my compiler patch at all.  BPF JIT backends may need to call the same runtime hooks that this toolchain patch generates jmps to, as provided by the runtime environment (in this case, the Linux kernel).

Also, it looks like work has started on backporting the kernel patches to the 5.18.y, 5.17.y, and 5.15.y branches of the LTS stable kernel tree.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/
(Not pushed publicly, just pointing where that tree is)
Requests have been made for 5.14.y and 4.19.y but it's not clear to me that work has started yet on those branches, or that it will.  Distros shipping kernels based on older branches will be encouraged to move to something newer, and something based off LTS (as usual)(some distros DON'T use LTS).

Thanks Nick for the details!

Thanks for reporting this to us in advance!

I'm representing Rust here, and from what I gather these mitigations are only going to be needed by the Linux kernel. In that case there shouldn't be a need to make a security point release of rustc, as Rust is not yet used in mainline Linux. Is my reading here correct?

> only going to be needed by the Linux kernel

For the time being, yes.  Other operating systems may choose to mitigate the issue similarly, or not at all.  I guess there could be another OS, written in Rust, that wants to mitigate the issue using this feature. But I'd perhaps wait for such a feature request from such theoretical developers if such a scenario exists.

> there shouldn't be a need to make a security point release of rustc, as Rust is not yet used in mainline Linux

The rust-for-linux folks might be interested in this eventually, but rustc would need feature development since the LLVM IR function attributes added here are controlled by command line flags and function attributes from clang and the C/C++ code respectively. Rust support isn't in the kernel yet, though I am hopeful it will be, and have been providing review upstream on their patch series.

So without the corresponding front end work to rustc, rolling my llvm+clang change out in a point release wouldn't be useful at this time, IMO.

Sounds good, thanks for the clarification! We'll avoid preparing a Rust point release then, unless after the embargo other OS vendors express interest in such a release.

> Sounds good, thanks for the clarification!

Thanks for checking, our Rust team had pretty much the exact same question.

> zero-daying every other architecture is also not a super great outcome.

So I reached out to ARM folks on the encrypted mailing list, as well as the reporters.  I heard back from both.

ARM re-iterated that they cannot comment on competing uArch implementations. They recommended the researchers contact Apple directly.  ARM mentioned they do "brief" architectural licensees, but don't necessarily share the researchers' whitepaper.  They also mentioned the same mailing lists be joined that I mentioned in https://crbug.com/llvm/33#c18.

This is second hand paraphrasing from me though; I don't speak on behalf of ARM in any capacity.

The researchers mentioned they have not tested their exploit on m1/m2 macs, or researched other architectures.

I can endeavor to prod people and/or forward anything to SEAR if I'm given something to forward. Researchers are free to email me and have me forward to people if they feel the public facing email is a blackhole.

If they're super paranoid, they can also use iMessage: oliver@nerget.com 

iMessage is secure, I worked on the crypto myself, trust me :D

I've shared your two email addresses with the researcher:
- ohunt@apple.com
- oliver@nerget.com

Otherwise, I believe their email address is:
Johannes Wikner <kwikner@ethz.ch>

So I've talked to Johannes, and it sounds like they're happy to upload details to the google bug tracker, however I have no familiarity with the system so don't know how to grant the required access.
If someone from Google knows, would they be able to?

[Empty comment from Monorail migration]

Trying to CC johannes.wikner@gmail.com as that's associated with a google account. Being a person who works with technology, has worked on browsers, etc I failed to notice the CC field.

Hi there,

I'll hand you our paper and the pocs we made for Intel (Skylake-like), AMD (fam 17h, but likely 15h,16h,18h too) and ARM ThunderX2. As you will find, however, we don't mention ARM at all in the paper. The only ARM machine we have in our lab is a ThunderX2, and it seems to be vulnerable to all kinds of BTI Spectres under this user--kernel threat model. Because these CPUs don't seem have (or use?) the necessary SMC workaround or CSV2 feature, we assumed that they could not be representative for ARM.

As for the AMD and Intel ones, the end-to-end exploits are made specifically for the kernel build we carried out the research on, which was the latest ubuntu/focal kernel available at the time (5.8.0-63-generic).

Sorry Johannes, I missed the above update. I've got this tracked internally as rdar://96815394. It includes your ETH contact + PGP information, so theoretically Product Security should reach out to you reasonably quickly (it is 2am PST however so the people I can prod are still asleep :) ). If you haven't heard anything by I guess around 2am tomorrow in Swiss time.

I have landed my patch.
https://reviews.llvm.org/D129572
https://reviews.llvm.org/rG2240d72f15f3b7b9d9fb65450f9bf635fd310f6f

Thanks again to the LLVM Security Group members, Researchers, and Aaron and Craig.

The embargo has now lifted.

Any concern with making this thread public from either the researchers side or the Apple side?

I'd be ok with keeping this thread embargoed until we have a confirmation from Oliver that we would not be zero-daying Apple.

There's no concern on the Apple side, thanks all!

Patch is reviewed+landed, so it seems like the security bug did its job on coordination. Echo Nick's thanks to everyone who came together to make sure we had a good response.

Given above messages, and more than a week with no objections, I'm derestricting.

This issue was migrated from crbug.com/llvm/33?no_tracker_redirect=1

[Auto-CCs applied]