[Bug c/104816] New: -fcf-protection=branch should generate endbr instead of notrack jumps

[Bug c/104816] New: -fcf-protection=branch should generate endbr instead of notrack jumps

[joao at overdrivepizza dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

            Bug ID: 104816
           Summary: -fcf-protection=branch should generate endbr instead
                    of notrack jumps
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: joao at overdrivepizza dot com
  Target Milestone: ---

When -fcf-protection=branch is used, the compiler will generate jump tables
where the indirect jump is prefixed with the NOTRACK prefix, so it can jump to
non-ENDBR targets. Yet, for NOTRACK prefixes to work, the NOTRACK specific
enable bit must be set, what renders the binary broken on any environment where
this is not the case. In fact, having NOTRACK disabled was a design choice for
the Linux kernel CET support [https://lkml.org/lkml/2022/3/7/1068].

With the above, the compiler should generate jump tables with ENDBRs, for
proper correctness. And, if security regarding the additional ENDBRs is a
concern, the code can be explicitly compiled with -fno-jump-tables.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[joao at overdrivepizza dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #1 from Joao Moreira <joao at overdrivepizza dot com> ---
quick reproducer, just in case: https://godbolt.org/z/EaG3rhrnj

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |12.0
           Severity|normal                      |enhancement

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Documentation should probably also amended to reflect this limitation (or
-fcf-protection=branch should implicitely disable jump-tables).

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-03-07

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Richard Biener from comment #2)
> Documentation should probably also amended to reflect this limitation (or
> -fcf-protection=branch should implicitely disable jump-tables).

We should document this limitation and update -fno-jump-tables documentation:

'-fno-jump-tables'
     Do not use jump tables for switch statements even where it would be
     more efficient than other code generation strategies.  This option
     is of use in conjunction with '-fpic' or '-fPIC' for building code
     that forms part of a dynamic linker and cannot reference the
     address of a jump table.  On some targets, jump tables do not
     require a GOT and this option is not needed.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #4 from Andrew Cooper <andrew.cooper3 at citrix dot com> ---
I've worked around this in Xen with:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=9d4a44380d273de22d5753883cbf5581795ff24d
and 
https://lore.kernel.org/lkml/YiXpv0q88paPHPqF@hirez.programming.kicks-ass.net/
is pending for Linux.

IMO, it's an error that -fcf-protection=branch is not obeyed for jump tables,
and we don't want to end up in a situation where jump tables are unusable with
CET.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Andrew Cooper from comment #4)
> I've worked around this in Xen with:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;
> h=9d4a44380d273de22d5753883cbf5581795ff24d and 
> https://lore.kernel.org/lkml/YiXpv0q88paPHPqF@hirez.programming.kicks-ass.
> net/ is pending for Linux.
> 
> IMO, it's an error that -fcf-protection=branch is not obeyed for jump
> tables, and we don't want to end up in a situation where jump tables are
> unusable with CET.

Are you suggesting to add an option to generate jump table with ENDBR?

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[peterz at infradead dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #6 from peterz at infradead dot org ---
(In reply to H.J. Lu from comment #5)
> (In reply to Andrew Cooper from comment #4)
> > I've worked around this in Xen with:
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;
> > h=9d4a44380d273de22d5753883cbf5581795ff24d and 
> > https://lore.kernel.org/lkml/YiXpv0q88paPHPqF@hirez.programming.kicks-ass.
> > net/ is pending for Linux.
> > 
> > IMO, it's an error that -fcf-protection=branch is not obeyed for jump
> > tables, and we don't want to end up in a situation where jump tables are
> > unusable with CET.
> 
> Are you suggesting to add an option to generate jump table with ENDBR?

I would suggest having -fcf-protection=branch generate ENDBR for jump-tables
and never generate NOTRACK prefix. Then add a mode that allows NOTRACK
prefixes, perhaps -fcf-protection=branch,notrack.

IBT without NOTRACK is the strongest form; it would be daft to require
additional parameters for that.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #7 from Andrew Cooper <andrew.cooper3 at citrix dot com> ---
(In reply to H.J. Lu from comment #5)
> Are you suggesting to add an option to generate jump table with ENDBR?

Jump tables are a legitimate optimisation.  NOTRACK is a weakness in CET
protections, and fully hardened userspace (as well as kernels) will want to run
with MSR_{U,S}_CET.NOTRACK_EN=0.

There should be some future where jump tables can be used in combination with
NOTRACK_EN=0.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Joao Moreira from comment #0)
> When -fcf-protection=branch is used, the compiler will generate jump tables
> where the indirect jump is prefixed with the NOTRACK prefix, so it can jump
> to non-ENDBR targets. Yet, for NOTRACK prefixes to work, the NOTRACK
> specific enable bit must be set, what renders the binary broken on any
> environment where this is not the case. In fact, having NOTRACK disabled was
> a design choice for the Linux kernel CET support
> [https://lkml.org/lkml/2022/3/7/1068].
> 
> With the above, the compiler should generate jump tables with ENDBRs, for
> proper correctness. And, if security regarding the additional ENDBRs is a
> concern, the code can be explicitly compiled with -fno-jump-tables.

There is an undocumented option: -mcet-switch.  It does exactly what you
are looking for.  Currently it is off by default.  We can document it
and turn it on by default.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 52615
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52615&action=edit
A patch

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #52615|0                           |1
        is obsolete|                            |

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 52618
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52618&action=edit
The v2 patch

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:2f4f7de787e5844515d27b2269fc472f95a9916a

commit r13-744-g2f4f7de787e5844515d27b2269fc472f95a9916a
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Mar 11 12:51:34 2022 -0800

    x86: Document -mcet-switch

    When -fcf-protection=branch is used, the compiler will generate jump
    tables for switch statements where the indirect jump is prefixed with
    the NOTRACK prefix, so it can jump to non-ENDBR targets.  Since the
    indirect jump targets are generated by the compiler and stored in
    read-only memory, this does not result in a direct loss of hardening.
    But if the jump table index is attacker-controlled, the indirect jump
    may not be constrained by CET.

    Document -mcet-switch to generate jump tables for switch statements with
    ENDBR and skip the NOTRACK prefix for indirect jump.  This option should
    be used when the NOTRACK prefix is disabled.

            PR target/104816
            * config/i386/i386.opt: Remove Undocumented.
            * doc/invoke.texi: Document -mcet-switch.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[peterz at infradead dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

--- Comment #12 from peterz at infradead dot org ---
On Tue, May 24, 2022 at 04:06:08PM +0000, cvs-commit at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
> 
> --- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
> The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:2f4f7de787e5844515d27b2269fc472f95a9916a
> 
> commit r13-744-g2f4f7de787e5844515d27b2269fc472f95a9916a
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Fri Mar 11 12:51:34 2022 -0800
> 
>     x86: Document -mcet-switch
> 
>     When -fcf-protection=branch is used, the compiler will generate jump
>     tables for switch statements where the indirect jump is prefixed with
>     the NOTRACK prefix, so it can jump to non-ENDBR targets.  Since the
>     indirect jump targets are generated by the compiler and stored in
>     read-only memory, this does not result in a direct loss of hardening.
>     But if the jump table index is attacker-controlled, the indirect jump
>     may not be constrained by CET.

Notrack indirect jumps are fully susceptible to speculation attacks.

[Bug target/104816] -fcf-protection=branch should generate endbr instead of notrack jumps

[i at maskray dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816

Fangrui Song <i at maskray dot me> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |i at maskray dot me

--- Comment #13 from Fangrui Song <i at maskray dot me> ---
I created https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643303.html
before I realized that there is a trade-off between two modes.

* (current default, -mno-cet-switch) NOTRACK indirect jump + case handlers
without ENDBR, GCC -mno-cet-switch. Vulnerable to unconstrained indirect jump
and Branch Target Injection.
* (-mcet-switch) tracked indirect jump + case handlers with ENDBR. Increases
the number of gadgets. Whether they can be usefully exploited depends on the
program.

It seems that the majority of the opinions so far are about the concern of
NOTRACK, so enabling -mcet-switch by default perhaps still makes sense.
-fno-jump-tables isn't a bad choice if users are really concerned about the
gadgets...