Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211013181658.1020262-1-samitolvanen@google.com>
 <20211026201622.GG174703@worktop.programming.kicks-ass.net> <20211027120515.GC54628@C02TD0UTHF1T.local>
In-Reply-To: <20211027120515.GC54628@C02TD0UTHF1T.local>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 27 Oct 2021 14:22:27 +0200
Message-ID: <CAMj1kXEx10gC8eH7rV-GbZZj2M3uDue6HYsKb+A5J01zOxm_FA@mail.gmail.com>
Subject: Re: [PATCH v5 00/15] x86: Add support for Clang CFI
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        X86 ML <x86@kernel.org>, Kees Cook <keescook@chromium.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Sedat Dilek <sedat.dilek@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        linux-hardening@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        llvm@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 27 Oct 2021 at 14:05, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Oct 26, 2021 at 10:16:22PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 13, 2021 at 11:16:43AM -0700, Sami Tolvanen wrote:
> > > This series adds support for Clang's Control-Flow Integrity (CFI)
> > > checking to x86_64. With CFI, the compiler injects a runtime
> > > check before each indirect function call to ensure the target is
> > > a valid function with the correct static type. This restricts
> > > possible call targets and makes it more difficult for an attacker
> > > to exploit bugs that allow the modification of stored function
> > > pointers. For more details, see:
> > >
> > >   https://clang.llvm.org/docs/ControlFlowIntegrity.html
> >
> > So, if I understand this right, the compiler emits, for every function
> > two things: 1) the actual funcion and 2) a jump-table entry.
> >
> > Then, every time the address of a function is taken, 2) is given instead
> > of the expected 1), right?
>
> Yes, and we had to bodge around this with function_nocfi() to get the
> actual function address.
>
> Really there should be a compiler intrinsic or attribute for this, given
> the compiler has all the releveant information available. On arm64 we
> had to us inine asm to generate the addres...
>
> Taking a step back, it'd be nicer if we didn't have the jump-table shim
> at all, and had some SW landing pad (e.g. a NOP with some magic bytes)
> in the callees that the caller could check for. Then function pointers
> would remain callable in call cases, and we could explcitly add landing
> pads to asm to protect those. I *think* that's what the grsecurity folk
> do, but I could be mistaken.
>
> > But how does this work with things like static_call(), which we give a
> > function address (now a jump-table entry) and use that to write direct
> > call instructions?
> >
> > Should not this jump-table thingy get converted to an actual function
> > address somewhere around arch_static_call_transform() ? This also seems
> > relevant for arm64 (which already has CLANG_CFI supported) given:
> >
> >   https://lkml.kernel.org/r/20211025122102.46089-3-frederic@kernel.org
>
> Ugh, yeah, we'll need to do the function_nocfi() dance somewhere...
>

Sadly, that only works on symbol names, so we cannot use it to strip
CFI-ness from void *func arguments passed into the static call API,
unfortunately.

Also, function_nocfi() seems broken in the sense that it relies on the
symbol existing in the global namespace, which may not be true for
function symbols with static linkage, as they can be optimized away
entirely. I think the same might apply to function symbols with
external linkage under LTO.