Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 646076 - flag-o-matic.eclass strips -mindirect-branch=thunk -mindirect-branch-register -mfunction-return=thunk
Summary: flag-o-matic.eclass strips -mindirect-branch=thunk -mindirect-branch-register...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All All
: Normal normal with 3 votes (vote)
Assignee: Gentoo Toolchain Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 646758
  Show dependency tree
 
Reported: 2018-01-29 20:31 UTC by DJ Dunn
Modified: 2022-06-10 04:54 UTC (History)
11 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
glibc-build.log.0.xz (with spectre_v2 flags) (glibc-build.log.0.xz,491.97 KB, application/octet-stream)
2018-02-07 01:13 UTC, kfm
Details
glibc-build.log.1.xz (vanilla flags) (glibc-build.log.1.xz,495.39 KB, application/octet-stream)
2018-02-07 01:14 UTC, kfm
Details
emerge --info (kerframil) (emerge-info.txt,5.10 KB, text/plain)
2018-02-07 01:17 UTC, kfm
Details

Note You need to log in before you can comment on or make changes to this bug.
Description DJ Dunn 2018-01-29 20:31:20 UTC
Glibc build strips -mindirect-branch=thunk

Required for protection against Spectre varient 2
Comment 1 DJ Dunn 2018-01-29 21:01:26 UTC
also -mindirect-branch-register
Comment 2 DJ Dunn 2018-01-29 21:10:42 UTC
id take a guess that the flag-o-matic.eclass needs to be updated to allow for it?
Comment 3 Tomáš Mózes 2018-01-30 04:15:13 UTC
With regards to https://github.com/speed47/spectre-meltdown-checker/blob/master/README.md I assumed this parameter is only needed for kernel building.
Comment 4 Tomáš Mózes 2018-01-30 05:11:44 UTC
arch/x86/Makefile:

ifdef CONFIG_RETPOLINE
    RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
    ifneq ($(RETPOLINE_CFLAGS),)
        KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DRETPOLINE
    endif
endif


Running with kernel 4.14.15 and gcc 7.3.0 (without changing /etc/portage/make.conf):

CVE-2017-5753 [bounds check bypass] aka 'Spectre Variant 1'
* Mitigated according to the /sys interface:  NO  (kernel confirms your system is vulnerable)
> STATUS:  VULNERABLE  (Vulnerable)

CVE-2017-5715 [branch target injection] aka 'Spectre Variant 2'
* Mitigated according to the /sys interface:  YES  (kernel confirms that the mitigation is active)
* Mitigation 1
  * Kernel is compiled with IBRS/IBPB support:  NO 
  * Currently enabled features
    * IBRS enabled for Kernel space:  NO 
    * IBRS enabled for User space:  NO 
    * IBPB enabled:  NO 
* Mitigation 2
  * Kernel compiled with retpoline option:  YES 
  * Kernel compiled with a retpoline-aware compiler:  YES  (kernel reports full retpoline compilation)
  * Retpoline enabled:  YES 
> STATUS:  NOT VULNERABLE  (Mitigation: Full generic retpoline)

CVE-2017-5754 [rogue data cache load] aka 'Meltdown' aka 'Variant 3'
* Mitigated according to the /sys interface:  YES  (kernel confirms that the mitigation is active)
* Kernel supports Page Table Isolation (PTI):  YES 
* PTI enabled and active:  YES 
* Running as a Xen PV DomU:  NO 
> STATUS:  NOT VULNERABLE  (Mitigation: PTI)
Comment 5 DJ Dunn 2018-01-30 07:16:18 UTC
its not just the kernel, basically everything is going to have to get rebuilt eventually, the kernel uses -mindirect-branch=thunk-extern -mindirect-branch-register. Userspace needs -mindirect-branch=thunk -mindirect-branch-register to protect from spectre.
Comment 6 Tomáš Mózes 2018-01-30 08:00:36 UTC
Hm I thought the gcc patches only mitigated spectre v2 (https://www.phoronix.com/scan.php?page=news_item&px=GCC-7.3-Released) using -mindirect-branch=thunk-extern -mindirect-branch-register.

Where did you find that information regarding userspace compilation settings please?
Comment 7 DJ Dunn 2018-01-30 08:20:32 UTC
https://support.google.com/faqs/answer/7625886

from the above link 

Binaries with shared linkage

While our initial focus has been the protection of operating system and hypervisor-type targets, there are classes of user application for which this coverage is valuable.  In these cases, it should be called out that shared linkage and runtimes will lead to frequent additional interactions with indirect branches. Ubiquitous examples include the Program Link Table (PLT) and dynamically loaded standard libraries.  We will be publishing additional optimization notes and techniques for these cases.


as i understand it this only protects the kernel from getting the kernels memory from being read, but other programs can still steal data from each other.
Comment 8 DJ Dunn 2018-01-30 08:31:29 UTC
the difference between mindirect-branch=thunk-extern and mindirect-branch=thunk people seem to be confused about, but general consensus seems to be think-extern for kernel and kernel modules and thunk for userspace
Comment 9 DJ Dunn 2018-01-30 08:38:01 UTC
mindirect-branch=choice
Convert indirect call and jump with choice. The default is ‘keep’, which keeps indirect call and jump unmodified. ‘thunk’ converts indirect call and jump to call and return thunk. ‘thunk-inline’ converts indirect call and jump to inlined call and return thunk. ‘thunk-extern’ converts indirect call and jump to external call and return thunk provided in a separate object file. You can control this behavior for a specific function by using the function attribute indirect_branch. See Function Attributes.

Note that -mcmodel=large is incompatible with -mindirect-branch=thunk nor -mindirect-branch=thunk-extern since the thunk function may not be reachable in large code model.

so in order to protect userspace that is vulnerable you need to enable retpoline in userspace also because the retpoline in the kernel only protects the kernel. there seems to be some confusion about which option here is required for userspace, but the kernel uses thunk-inline
Comment 10 DJ Dunn 2018-01-30 08:38:47 UTC
oh the last post was mostly from the GCC documention about the -mindiract-branch=
Comment 11 Sergei Trofimovich (RETIRED) gentoo-dev 2018-01-30 20:22:25 UTC
The set of allowed flags is defined by flag-o-matic.eclass, not toolchain.eclass.  toolchain.eclass (and other sensitive packages that rely on predictable instruction sequences) only uses strip-flags when gcc is configured, not glibc.

https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/flag-o-matic.eclass#n24

These are known to be safe flags. Did you build glibc with various -mindirect-branch= enabled?
How many more tests started failing on glibc?

'FEATURES=test emerge -v1 glibc' to get them running.
Comment 12 DJ Dunn 2018-01-30 21:25:14 UTC
I know flag-o-matic is the eclass that strips, someone else put toolchain.eclass in.
The flag-o-matic is what's keeping me from trying glibc with these flags. I guess I have to hack flag-o-matic to allow these tests?
Comment 13 Sergei Trofimovich (RETIRED) gentoo-dev 2018-02-03 09:49:37 UTC
(In reply to DJ Dunn from comment #12)
> I guess I have to hack flag-o-matic to allow these tests?

Yes.
Comment 14 kfm 2018-02-06 13:39:08 UTC
In addition to -mindirect-branch and -mindirect-branch-register, please also consider -mfunction-return for addition to the ALLOWED_FLAGS array.
Comment 15 kfm 2018-02-06 14:06:27 UTC
The reason that the kernel uses "thunk-extern" is because it provides its own thunk handlers. See https://github.com/torvalds/linux/commit/76b043848fd22dbf7f8bf3a1452f8c70d557b860. It does no good for userspace in general, with "thunk" being the correct choice there.

I'm currently using CFLAGS="-mfunction-return=thunk -mindirect-branch=thunk -mindirect-branch-register" for my userspace, having monkey-patched flag-o-matic.eclass to this end. I'll re-build glibc with tests enabled, and post the details once done.
Comment 16 kfm 2018-02-07 01:09:34 UTC
I attempted to build glibc-2.25-r9 with FEATURES=test, and the build fails. However, the failures are not due to my use of the additional CFLAGS. I'm attaching my build logs, in case anyone is interested. Here are the details of the CFLAGS employed (after being processed by strip-flags):-

glibc-build.log.0:
    CFLAGS="-pipe -march=westmere -mfunction-return=thunk -mindirect-branch=thunk -mindirect-branch-register -O2

glibc-build.log.1:
    CFLAGS="-pipe -march=westmere -O2"

Both builds resulted in the same set of failures.

I am happy to perform further test builds, if so instructed. Otherwise, I can only report that my build of glibc - along with all the other packages that I use - are functioning without any apparent issue, and have all been built with the flags that are the subject of this bug.
Comment 17 kfm 2018-02-07 01:13:08 UTC
Created attachment 518294 [details]
glibc-build.log.0.xz (with spectre_v2 flags)
Comment 18 kfm 2018-02-07 01:14:07 UTC
Created attachment 518296 [details]
glibc-build.log.1.xz (vanilla flags)
Comment 19 kfm 2018-02-07 01:17:41 UTC
Created attachment 518298 [details]
emerge --info (kerframil)
Comment 20 Sergei Trofimovich (RETIRED) gentoo-dev 2018-02-07 07:55:56 UTC
(In reply to Kerin Millar from comment #16)
> I attempted to build glibc-2.25-r9 with FEATURES=test, and the build fails.
> However, the failures are not due to my use of the additional CFLAGS.

I suggest trying at least glibc-2.26. Those have mostly working tests.
Comment 21 Eddie Chapman 2018-02-07 09:30:37 UTC
I've just rebuilt everything with glibc 2.27 and -mindirect-branch=thunk without any problems (other than those of certain individual packages already known about). I've been running several production Gentoo servers with -mindirect-branch=thunk for 3 weeks now without any issues.

I guess as the weeks go by it will become clear what packages this is really unnecessary for, but for now I'm applying it to everything to be safe. 

Whilst it is pretty clear that -mindirect-branch=thunk is required for userspace (and thunk-extern for kernel), so far I've been unable to find any authoritative source of info (e.g. mailing list post, commit log) confirming when and if -mindirect-branch-register and/or -mfunction-return=thunk are required. I've seen "rumour" that these are only required for newer cpu, or for those models that receive a microcode update. Has anyone come to any firm conclusion about this?
Comment 22 Eddie Chapman 2018-02-07 09:32:09 UTC
To clarify, by "problems" I meant ones with glibc 2.27, not -mindirect-branch=thunk
Comment 23 kfm 2018-02-07 10:30:18 UTC
(In reply to Eddie Chapman from comment #21)
...

> Whilst it is pretty clear that -mindirect-branch=thunk is required for
> userspace (and thunk-extern for kernel), so far I've been unable to find any
> authoritative source of info (e.g. mailing list post, commit log) confirming
> when and if -mindirect-branch-register and/or -mfunction-return=thunk are
> required. I've seen "rumour" that these are only required for newer cpu, or
> for those models that receive a microcode update. Has anyone come to any
> firm conclusion about this?

If you look at the dependency tree for this bug, you'll see that I attempted to address this confusion in both of the parent bugs. A decent overview of how retpolines work was posted by Tobias Ribizel at stackoverflow [1], which some may find easier to digest than the Google papers. Further, kernel developer, David Woodhouse wrote an informative post, concerning the various mitigation techniques that have been devised [2].

As noted by David, retpolines are an effective counter to Spectre V2, but are insufficient for the Skylake microarchitecture and its successors. Thus, the "rumour" you refer to is backwards. The newer CPUs are in need of microcode updates in order to leverage the other techniques that are described, while the implementation of a retpoline is accomplished exclusively in software.

The initial post of the gcc patchset by H.J. Lu [3] shows that all of the CFLAGS under consideration here pertain to Spectre V2 mitigation. Note, however, that -mindirect-branch-loop does not exist in gcc-7.3, but will land in gcc-8.0. I don't claim to understand the mechanics exactly, but I have never seen -mindirect-branch=* used without -mindirect-branch-register, and the kernel is no exception. From what I can gather, the latter flag coerces gcc into employing the necessary indirection.

[1] https://stackoverflow.com/a/48099456
[2] https://lkml.org/lkml/2018/1/22/598
[3] https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00422.html
Comment 24 kfm 2018-02-07 10:31:29 UTC
(In reply to Sergei Trofimovich from comment #20)
> (In reply to Kerin Millar from comment #16)
> > I attempted to build glibc-2.25-r9 with FEATURES=test, and the build fails.
> > However, the failures are not due to my use of the additional CFLAGS.
> 
> I suggest trying at least glibc-2.26. Those have mostly working tests.

Thanks, Sergei. I'll give that a try.
Comment 25 Eddie Chapman 2018-02-07 11:27:37 UTC
Thanks Kerin.

(In reply to Kerin Millar from comment #23)
> If you look at the dependency tree for this bug, you'll see that I attempted
> to address this confusion in both of the parent bugs. A decent overview of
> how retpolines work was posted by Tobias Ribizel at stackoverflow [1], which
> some may find easier to digest than the Google papers. Further, kernel
> developer, David Woodhouse wrote an informative post, concerning the various
> mitigation techniques that have been devised [2].

I have read most of that thread before, but it just confirms what I already concluded 3 weeks ago, that retpoline is the best approach to mitigate against Spectre V2 at the moment. No discussion of the specific GCC flags there as far as I can see.

> As noted by David, retpolines are an effective counter to Spectre V2, but
> are insufficient for the Skylake microarchitecture and its successors. Thus,
> the "rumour" you refer to is backwards. The newer CPUs are in need of
> microcode updates in order to leverage the other techniques that are
> described, while the implementation of a retpoline is accomplished
> exclusively in software.

The "rumours" I've seen (i'd prefer a better word, can't think of one) are not about whether or not retpoline is needed, or its merits compared to other approaches. They are about when and if -mindirect-branch-register and/or -mfunction-return should be used when building userspace (always in conjunction with -mindirect-branch=thunk of course). I have seen people saying they have decided they do not need these flags due to having older non-Skylake CPU, and people who are obviously using them, but I haven't found any authoritative (*and* understandable by mere mortals) direction yet about the use of these flags specifically.

> Note however, that -mindirect-branch-loop does not exist in gcc-7.3, but will
> land in gcc-8.0.

Thanks, hadn't seen that, useful to know.

> I have never seen -mindirect-branch=* used without -mindirect-branch-register,
> and the kernel is no exception. From what I can gather, the latter flag
> coerces gcc into employing the necessary indirection.

Well, part of the problem is it's hard to find examples of usage of -mindirect-branch-register and/or -mfunction-return online (apart from the kernel). I've been trying to find userspace examples from distro packages but so far have only seen them releasing updated gcc packages but nothing (apart from the kernel) actually built with retpoline, when I last checked at least.

I've read H.J. Lu's original patch posts a few times the last month but they are very difficult for anyone other than GCC and/or kernel developers to get their head around. Particularly here are the 2 original, individual patches for -mfunction-return and -mindirect-branch-register respectively:

https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00426.html
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00427.html

I am unable to decipher conclusively from these 2 posts when/if these flags should be used.
Comment 26 kfm 2018-02-07 14:51:49 UTC
Taking the (In reply to Eddie Chapman from comment #25)

...

> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00426.html
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00427.html
> 
> I am unable to decipher conclusively from these 2 posts when/if these flags
> should be used.

My understanding of x86 assembler is rudimentary, at best, but let's take -mfunction-return as an example. Within a given sequence of opcodes that constitute a function, rather than terminating with ret, an instruction is inserted to jump to the address of where the thunk is implemented. This thunk is, in fact, the implementation of a return trampoline. Within this thunk, the L1 symbolic label is used as a target for a jmp instruction, thus constituting an infinite loop. However, this is never 'directly' executed because it is effectively leap-frogged by calling into L2. Essentially, the CPU is tricked into speculatively executing the (intentionally useless) L1 path, with the LFENCE instruction used to prevent unnecessary CPU burn.

Here's the important thing, though. If you don't specify -mfunction-return=thunk, then gcc does not inject its __x86_return_thunk code, nor does it modify function code to jump to this thunk as a mechanic for returning. In essence, if you don't ask for retpolines, then you don't get them. From this, you should be able to infer that that, if you are not enabling all of the gcc options that implement its supported thunks, then you are not achieving maximal generic protection against Spectre V2 in your userspace.

Going back to the topic, whitelisting these flags in flag-o-matic would mean that users who wish to trade some performance in favour of security would be able to apply these flags to ebuilds that use the strip-flags function, of which the most important one is glibc.
Comment 27 Eddie Chapman 2018-02-07 18:46:47 UTC
(In reply to Kerin Millar from comment #26)
> Here's the important thing, though. If you don't specify
> -mfunction-return=thunk, then gcc does not inject its __x86_return_thunk
> code, nor does it modify function code to jump to this thunk as a mechanic
> for returning. In essence, if you don't ask for retpolines, then you don't
> get them. From this, you should be able to infer that that, if you are not
> enabling all of the gcc options that implement its supported thunks, then
> you are not achieving maximal generic protection against Spectre V2 in your
> userspace.

Yes, this is kind of off-topic, I apologise if anyone is annoyed by this. However, I suspect those interested in this bug report also happen to be interested in the use of these particular flags, so please allow me one more post.

Whilst what you've written about mfunction-return=thunk seems to me to make sense, in fact I'm not so sure if it *is* required when building userspace in *all* scenarios. I've just stumbled upon something else interesting, after looking at [1] where these flags are discussed.

Apparently, mfunction-return=thunk was requested specifically by the kernel devs [2] only for future use. This is further confirmed by David Woodhouse who states that "*some* CPUs also pull in predictions from the generic branch target predictor when the return stack buffer has underflowed (e.g. if there was a call stack of more than X depth). Hence, in some cases we may yet end up needing this -mfunction-return=thunk too. As you (Martin) note, we don't use it *yet*. The full set of mitigations for the various attacks are still being put together, and the optimal choice for each CPU family does end up being different." [3]

So I'm suspecting that mfunction-return=thunk might only be needed for Skylake onwards. In my case all the boxes are pre-Skylake.

This would make sense because it seemed ridiculous to me that the GCC developers would introduce an option (mindirect-branch) which is effectively a no-op by itself.

[1] https://stackoverflow.com/questions/48428351/how-to-mitigate-spectre-with-gcc-and-clang-or-llvm-in-general
[2] https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00469.html
[3] https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00540.html
Comment 28 kfm 2018-02-07 20:12:24 UTC
(In reply to Eddie Chapman from comment #27)

That is interesting, and your reasoning makes sense to me. While I am only obligated to worry about Westmere through Sandy Bridge, the combination of uncertainty and having CPU cycles to spare will keep me using -mfunction-return - along with the others - for the time being. Besides which, it may be useful for someone to be testing them all for potential regressions. Even so, I would very much like to be able to draw a firm conclusion as to which combinations are ideal for a given target CPU.
Comment 29 Arfrever Frehtes Taifersar Arahesis 2018-05-29 10:01:06 UTC
(In reply to Kerin Millar from comment #23)
> The initial post of the gcc patchset by H.J. Lu [3] shows that all of the
> CFLAGS under consideration here pertain to Spectre V2 mitigation. Note,
> however, that -mindirect-branch-loop does not exist in gcc-7.3, but will
> land in gcc-8.0.

The patch adding -mindirect-branch-loop=* option was discarded after discussion on gcc-patches mailing list.
This option is not in GCC 8.
This option was used to control which instruction was loop filler.
"lfence" is preferred by AMD.
"pause" is preferred by Intel.
Currently output_indirect_thunk() function contains:
  /* AMD and Intel CPUs prefer each a different instruction as loop filler.
     Usage of both pause + lfence is compromise solution.  */
  fprintf (asm_out_file, "\tpause\n\tlfence\n");
Comment 30 Sergei Trofimovich (RETIRED) gentoo-dev 2020-11-15 22:31:25 UTC
Closing as WONTFIX as there was no test run against recent glibc.
Comment 31 Arfrever Frehtes Taifersar Arahesis 2020-11-16 14:47:47 UTC
I have made test runs against recent glibc.
With these flags, there are no additional test failures in glibc.
Comment 32 Sergei Trofimovich (RETIRED) gentoo-dev 2020-11-16 17:55:23 UTC
(In reply to Arfrever Frehtes Taifersar Arahesis from comment #31)
> I have made test runs against recent glibc.
> With these flags, there are no additional test failures in glibc.

Please attach used patch and I'll apply it with 'git am'.
Comment 33 Larry the Git Cow gentoo-dev 2022-01-22 22:19:04 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=55065b06c6d9ea69732624565caca2ac10320543

commit 55065b06c6d9ea69732624565caca2ac10320543
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2022-01-18 16:41:43 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2022-01-22 22:18:53 +0000

    flag-o-matic.eclass: allow Spectre mitigation flags
    
    Closes: https://bugs.gentoo.org/646076
    Signed-off-by: Sam James <sam@gentoo.org>

 eclass/flag-o-matic.eclass | 5 +++++
 1 file changed, 5 insertions(+)