Message-ID: <CAMe9rOoHxJixg40r-3mnYeC-GQYj1gqwY9S_iMCdgc-DaajzCQ@mail.gmail.com>
Date:   Thu, 21 Apr 2022 15:26:34 -0700
From:   "H.J. Lu" <hjl.tools@...il.com>
To:     Fangrui Song <maskray@...gle.com>
Cc:     Joao Moreira <joao@...rdrivepizza.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-hardening@...r.kernel.org,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        Kees Cook <keescook@...omium.org>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Mark Rutland <mark.rutland@....com>,
        alyssa.milburn@...ux.intel.com, gabriel.gomes@...ux.intel.com,
        Rick P Edgecombe <rick.p.edgecombe@...el.com>
Subject: Re: [RFC PATCH 00/11] Kernel FineIBT Support

On Thu, Apr 21, 2022 at 3:11 PM Fangrui Song <maskray@...gle.com> wrote:
>
> On 2022-04-21, H.J. Lu wrote:
> >On Thu, Apr 21, 2022 at 8:23 AM Joao Moreira <joao@...rdrivepizza.com> wrote:
> >>
> >> On 2022-04-21 00:49, Peter Zijlstra wrote:
> >> > On Wed, Apr 20, 2022 at 03:40:41PM -0700, Joao Moreira wrote:
> >> >> > >
> >> >> > > If FineIBT needs it, I could reconsider.  But I think there's a strong
> >> >> > > case to be made that the linker should be doing that instead.
> >> >> >
> >> >> > That sounds reasonable to me (and reminds me of linker relaxation).
> >> >> > Joao, can you please work with Fangrui (LLD) and HJ (GNU binutils) to
> >> >> > determine how feasible this would be? I assume code outside the kernel
> >> >> > might enjoy such an optimization, too.  When that's the case, then it
> >> >> > probably makes more sense to "upstream" such "optimizations" from the
> >> >> > kernel-specific objtool into the toolchains.
> >> >>
> >> >> Alright, these are the greenlights I was hoping for.
> >> >>
> >> >> I went quickly into this with HJ and he mentioned that it should be
> >> >> doable
> >> >> in the linker, and that he has a patch for it in gcc (for local
> >> >> function,
> >> >> from what I could see):
> >> >> https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590832.html
> >> >>
> >> >> If @Fangrui is fine with it, I would like to try implementing this
> >> >> myself in
> >> >> lld (I'm still learning a lot about lld and having an actual problem
> >> >> to
> >> >> solve is the kind of fuel I need). Should take me a while, but I think
> >> >> this
> >> >> is not urgent, right? I can also go ahead and replicate HJ's gcc patch
> >> >> into
> >> >> clang, so we can also handle the local functions within the compiler
> >> >> (I
> >> >> think this makes a lot of sense).
> >> >>
> >> >> Once we have these in, I'll revisit FineIBT and extend the features to
> >> >> handle the FineIBT instrumentation. Hopefully we'll be released from
> >> >> needing
> >> >> objtool (famous last words?!).
> >> >>
> >> >> This sounds like a plan, but I'm ofc open to suggestions or different
> >> >> ideas/plans.
>
> Thanks for looping me in! (I mean: this is discussed beforehand, instead
> of GCC/GNU ld taking some steps and pushing LLVM toolchain to follow
> suite..)
> Though I think I may not have the full context here...
>
> I can see that you have a request to skip the endbr instruction
> (and potentially more instructions for FineIBT).
>
> The GCC patch https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590832.html
> achieves it. A linker implementation will handle more cases.
> This is somewhat similar to PowerPC64's global entry vs local entry.
> The linker can redirect a branch instruction to the local entry in some
> conditions. My concern is that inspecting the section content goes too far
> and breaks the spirit of ELF relocation resolving. The proper way is to use
> a symbol attribute (e.g. st_other).
>
> st_other bits are scarce, so any use needs to be prudent.

On the other hand, linker inspection doesn't require changes in object files.
It is much more user friendly.   X86-64 psABI already allows linker optimization
based on contents at relocation sites.   This goes one step further to contents
at relocation targets.

> >> > So trivially the plan sounds like: compiler fixes STB_LOCAL because it
> >> > has the scope, and the linker fixes everything else. However, that
> >> > seems
> >> > to assume that !STB_LOCAL will have ENDBR.
> >> >
> >> > This latter isn't true; for one there's __attribute__((nocf_check))
> >> > that
> >> > can be used to suppress ENDBR generation on a function.
> >> >
> >> > Alternatively the linker will need to 'read' the function to determine
> >> > if it has ENDBR, or we need to augment the ELF format such that we can
> >> > tell from that.
> >> >
> >> > So what exactly is the plan?
> >>
> >> I ran into too many broken dreams by trying to infer the presence of
> >> ENDBRs just by the symbol locality/linkage... not only because of the
> >> attribute, but also because of ancient assembly.
> >>
> >> So, my first thought was to use something similar to the
> >> __patchable_function_entries section
> >> (https://man7.org/linux/man-pages/man1/gcc.1.html), where we would have
> >> a section to mark all the placed ENDBR. But then it occurred to me that
> >> if we follow that road we'll miss the ENDBR placed in assembly unless we
> >> mark it manually, so I started thinking that reading the target
> >> instructions from the ELF object could be a more simplified approach,
> >> although a little more treacherous.
> >>
> >> I didn't decide yet what to try first -- any thoughts?
> >>
> >> @Fangrui's and @HJ's thoughts about this could be gold.
> >
> >You can't assume ENDBR existence just by symbol visibility.
> >Compiler knows if there is an ENDBR at function entry since
> >it is generated by compiler.   Otherwise, you need to check
> >the first 4 bytes at function entry,
> >
> >--
> >H.J.



-- 
H.J.

Powered by blists - more mailing lists