mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joao Moreira <joao@overdrivepizza.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	peterz@infradead.org, andrew.cooper3@citrix.com,
	keescook@chromium.org, samitolvanen@google.com,
	mark.rutland@arm.com, hjl.tools@gmail.com,
	alyssa.milburn@linux.intel.com, ndesaulniers@google.com,
	gabriel.gomes@linux.intel.com, rick.p.edgecombe@intel.com
Subject: Re: [RFC PATCH 01/11] x86: kernel FineIBT
Date: Tue, 03 May 2022 19:19:11 -0700	[thread overview]
Message-ID: <068cafedb0699383b696ce15d5677979@overdrivepizza.com> (raw)
In-Reply-To: <20220503220244.vyz5flk3gg3y6rbw@treble>

> 
> 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 :)

  reply	other threads:[~2022-05-04  2:19 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20  0:42 [RFC PATCH 00/11] Kernel FineIBT Support joao
2022-04-20  0:42 ` [RFC PATCH 01/11] x86: kernel FineIBT joao
2022-04-29  1:37   ` Josh Poimboeuf
2022-05-02 17:17     ` Joao Moreira
2022-05-03 22:02       ` Josh Poimboeuf
2022-05-04  2:19         ` Joao Moreira [this message]
2022-05-04 10:20         ` Peter Zijlstra
2022-05-04 17:04           ` Peter Collingbourne
2022-05-04 18:16             ` Peter Zijlstra
2022-05-05  0:28               ` Sami Tolvanen
2022-05-05  7:36                 ` Peter Zijlstra
2022-05-08  8:29               ` Kees Cook
2022-05-09 11:22                 ` Peter Zijlstra
2022-04-20  0:42 ` [RFC PATCH 02/11] kbuild: Support FineIBT build joao
2022-04-20  0:42 ` [RFC PATCH 03/11] objtool: Support FineIBT offset fixes joao
2022-04-20  0:42 ` [RFC PATCH 04/11] x86/module: Support FineIBT in modules joao
2022-04-20  0:42 ` [RFC PATCH 05/11] x86/text-patching: Support FineIBT text-patching joao
2022-04-20  0:42 ` [RFC PATCH 06/11] x86/bpf: Support FineIBT joao
2022-04-20  0:42 ` [RFC PATCH 07/11] x86/lib: Prevent UACCESS call warning from objtool joao
2022-04-20  0:42 ` [RFC PATCH 08/11] x86/ibt: Add CET_TEST module for IBT testing joao
2022-04-20  0:42 ` [RFC PATCH 09/11] x86/FineIBT: Add FINEIBT_TEST module joao
2022-04-20  0:42 ` [RFC PATCH 10/11] linux/interrupt: Fix prototype matching property joao
2022-04-20  2:45   ` Kees Cook
2022-04-20 22:14     ` Joao Moreira
2022-04-20  0:42 ` [RFC PATCH 11/11] driver/int3400_thermal: Fix prototype matching joao
2022-04-20  2:55   ` Kees Cook
2022-04-20 22:28     ` Joao Moreira
2022-04-20 23:04       ` Kees Cook
2022-04-20 23:12         ` Joao Moreira
2022-04-20 23:25           ` Kees Cook
2022-04-21  0:28             ` Joao Moreira
2022-04-20  2:42 ` [RFC PATCH 00/11] Kernel FineIBT Support Kees Cook
2022-04-20 22:50   ` Joao Moreira
2022-04-20  7:40 ` Peter Zijlstra
2022-04-20 15:17   ` Josh Poimboeuf
2022-04-20 17:12     ` Nick Desaulniers
2022-04-20 22:40       ` Joao Moreira
2022-04-21  7:49         ` Peter Zijlstra
2022-04-21 15:23           ` Joao Moreira
2022-04-21 15:35             ` H.J. Lu
2022-04-21 22:11               ` Fangrui Song
2022-04-21 22:26                 ` H.J. Lu
2022-04-20 23:34 ` Edgecombe, Rick P

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=068cafedb0699383b696ce15d5677979@overdrivepizza.com \
    --to=joao@overdrivepizza.com \
    --cc=alyssa.milburn@linux.intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=gabriel.gomes@linux.intel.com \
    --cc=hjl.tools@gmail.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=samitolvanen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®