Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220429203644.2868448-1-samitolvanen@google.com>
 <202204291545.47C6A97EA2@keescook> <Ymz7DWkRJy3PMaHm@hirez.programming.kicks-ass.net>
 <CABCJKuckt75qA1op-LpkJnQsJC36m9fstbY3uD=7pET2VyyZSg@mail.gmail.com> <YnA3Le2uJRaFF4TC@hirez.programming.kicks-ass.net>
In-Reply-To: <YnA3Le2uJRaFF4TC@hirez.programming.kicks-ass.net>
From:   Peter Collingbourne <pcc@google.com>
Date:   Tue, 3 May 2022 15:35:34 -0700
Message-ID: <CAMn1gO614sMzKzP6qTCcXB0QCeEu-G7CHn_h=6ikCLkFo_Z2Dg@mail.gmail.com>
Subject: Re: [RFC PATCH 00/21] KCFI support
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Sami Tolvanen <samitolvanen@google.com>,
        Kees Cook <keescook@chromium.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Joao Moreira <joao@overdrivepizza.com>,
        Sedat Dilek <sedat.dilek@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
        linux-hardening@vger.kernel.org,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        llvm@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, May 2, 2022 at 1:02 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, May 02, 2022 at 08:22:57AM -0700, Sami Tolvanen wrote:
>
> > > Anyway, I think I hate that __builtin, I'd *much* rather see a variable
> > > attribute or qualifier for this, such that one can mark a function
> > > pointer as not doing CFI.
> > >
> > > I simply doesn't make sense to have a builtin that operates on an
> > > expression. The whole thing is about indirect calls, IOW function
> > > pointers.
> >
> > I also thought an attribute would be more convenient, but the compiler
> > folks prefer a built-in:
> >
> > https://reviews.llvm.org/D122673
>
> That seems to mostly worry about C++ things (overload sets, template
> specialization, name mangling) we kernel folks don't seem to much care
> about.
>
> I'll stick with saying type system makes more sense to me though.

I'd say it's not only the C++ issues but more the "action at a
distance" that's implied by having this be part of the type system.
With this being in the function type it's hard to tell whether any
particular call will have CFI disabled, without needing to go and look
at how the function pointer is defined. On the other hand, if we
explicitly mark up the calls with CFI disabled, the code becomes
easier to audit (think Rust "unsafe" blocks).

Does it seem any better to you to have this be marked up via the
function expression, rather than the call? The idea is that this would
always compile to a check-free function call, no matter what "func"
is:

__builtin_kcfi_call_unchecked(func)(args)

We already have this, to some degree, with KCFI as implemented: CFI
checks are disabled if the function expression refers to a declared
function. The builtin would allow overriding the decision to also
disable CFI checks for function expressions that use the builtin. It
also wouldn't preclude a type based system later on (the builtin would
become effectively a cast to the "unchecked" type).

Peter