On Sat, Apr 30, 2022 at 2:02 AM Peter Zijlstra <[email protected]> wrote:
>
> On Fri, Apr 29, 2022 at 03:53:12PM -0700, Kees Cook wrote:
> > On Fri, Apr 29, 2022 at 01:36:23PM -0700, Sami Tolvanen wrote:
> > > KCFI is a proposed forward-edge control-flow integrity scheme for
> > > Clang, which is more suitable for kernel use than the existing CFI
> > > scheme used by CONFIG_CFI_CLANG. KCFI doesn't require LTO, doesn't
> > > alter function references to point to a jump table, and won't break
> > > function address equality.
> >
> > ???? :)
> >
> > > The latest LLVM patches are here:
> > >
> > > https://reviews.llvm.org/D119296
> > > https://reviews.llvm.org/D124211
> > >
> > > [...]
> > > To test this series, you'll need to compile your own Clang toolchain
> > > with the patches linked above. You can also find the complete source
> > > tree here:
> > >
> > > https://github.com/samitolvanen/llvm-project/commits/kcfi-rfc
> >
> > And note that this RFC is seeking to break a bit of a circular dependency
> > with regard to the design of __builtin_kcfi_call_unchecked (D124211
> > above), as the implementation has gone around a few times in review within
> > LLVM, and we want to make sure that kernel folks are okay with what was
> > settled on. If there are no objections on the kernel side, then we can
> > land the KCFI patches, as this is basically the only remaining blocker.
>
> So aside from the static_call usage, was there any other?
Not at the moment, and it looks like we can get rid of that too.
> 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
Sami
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.
On Mon, May 2, 2022 at 1:02 PM Peter Zijlstra <[email protected]> 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