Re: [RFC PATCH 01/11] x86: kernel FineIBT

Joao Moreira
2022-05-04T02:19:25Z

>
> It should be possible to have a non-fatal #UD2 handler.
>
> See for example how WARN() is implemented with __WARN_FLAGS in
> arch/x86/include/asm/bug.h .
>
> So hopefully we can just get rid of the need for the "call handler"
> thing altogether.
>
Nice, I'll look into it. Tks.

>> > Not sure what would happen for "ibt=off"? Maybe apply_ibt_endbr() could
>> > NOP out all the FineIBT stuff.
>>
>> Either that, or...
>>
>> I'm thinking about a way to have FineIBT interchangeable with KCFI.
>> Currently KCFI adds a 4 byte hash + 2 byte nops before function entry,
>> to
>> allow for proper prototype checking. After that, there should be an
>> ENDBR of
>> 4 bytes. This gives us 10 bytes in total. Then, my yet to be properly
>> thought idea would be patch these 10 bytes with:
>>
>> endbr
>> call fineibt_handler_<$HASH>
>> nop
>>
>> and then, on the caller side, patch the "cmp <$HASH>, -0x6(%r11); je;
>> ud2;
>> call" sequence with a "sub 0x6, r11; mov $HASH, %r10; call %r11, add
>> 0x6
>> %r11". This would then allow the kernel to verify if the CPU is IBT
>> capable
>> on boot time and only then setting the proper scheme.
>>
>> The downsides of having something like this would be that this sub
>> r11/add
>> r11 sequence is kinda meh. We can avoid that by having two padding
>> nops
>> after the original ENDBR, which will be skipped when the function is
>> reached
>> directly by the linker optimization I'm working on, and that we can
>> convert
>> into a JMP -offset that makes control flow reach the padding area
>> before the
>> prologue and from where we can call the fineibt_handler function. The
>> resulting instrumentation would be something like:
>>
>> 1:
>> call fineibt_handler_<$HASH>
>> jmp 2f
>> <foo>
>> endbr
>> jmp 1b
>> 2:
>>
>> Also, it would prevent a paranoid user to have both schemes
>> simultaneously
>> (there are reasons why people could want that).
>>
>> Any thoughts?
>
> I'm not really qualified to comment on this too directly since I
> haven't
> looked very much at the variations on FineIBT/CFI/KCFI, and what the
> protections and drawbacks are for each approach, and when it might even
> make sense to combine them for a "paranoid user".
>
> Since we have multiple similar and possibly competing technologies
> being
> discussed, one thing I do want to warn against is that we as kernel
> developers tend to err on the side of giving people too many choices
> and
> combinations which *never* get used.
>
> All those unused options can confuse the user and significantly add to
> complexity and maintenance overhead for us. Especially for invasive
> features like these.
>
> (Just to be clear, I'm not saying that's happening here, but it's
> something we need to be careful about.)
>
> Here, documentation is going to be crucial, for both reviewers and
> users. Something that describes when/why I should use X or Y or X+Y.
>
> If we truly want to add more options/combos for different use cases
> then
> we'll also need clear and concise documentation about which
> options/combos would be used under what circumstances.

Yeah, I totally understand/support this concern and I feel the same way.
While, in this case, I can't see super clear reasons for X+Y, there are
aspects why someone could prefer X or Y -- so I think that using
alternatives to flip the instrumentation is a valid consideration. In
time, taking the chance to be fair on the credits, using alternatives to
replace KCFI/FineIBT was also Peter's idea, not mine. It looked hard to
do at first sight because of the caller/callee-side checks differences,
but since Peter mentioned it, I started trying to solve the puzzle of
having the best suitable instrumentation that would be changeable. I
haven't discussed this with anyone yet, but at this point I think it
might be doable, although not in the most performant shape. Anyway, I'll
post something here once I have a more solid idea.

And yes, I agree that documentation will be key and I totally see your
point/understand how confusing I was in my previous e-mail. I'll keep
that in mind for the next revision. Thanks for pointing it out :)