Re: CET/IBT support and live-patches

On 11/23/21 4:58 AM, Miroslav Benes wrote:

[ adding more CCs ]

On Mon, 22 Nov 2021, joao@overdrivepizza.com wrote:

quoted

Hi Miroslav, Petr and Nicolai,

Long time no talk, I hope you are all still doing great :)

Everything great here :)

quoted

So, we have been cooking a few patches for enabling Intel CET/IBT support in
the kernel. The way IBT works is -- whenever you have an indirect branch, the
control-flow must land in an endbr instruction. The idea is to constrain
control-flow in a way to make it harder for attackers to achieve meaningful
computation through pointer/memory corruption (as in, an attacker that can
corrupt a function pointer by exploiting a memory corruption bug won't be able
to execute whatever piece of code, being restricted to jump into endbr
instructions). To make the allowed control-flow graph more restrict, we are
looking into how to minimize the number of endbrs in the final kernel binary
-- meaning that if a function is never called indirectly, it shouldn't have an
endbr instruction, thus increasing the security guarantees of the hardware
feature.

Some ref about what is going on --
https://lore.kernel.org/lkml/20211122170805.149482391@infradead.org/T/ (local)

Yes, I noticed something was happening again. There was a thread on this 
in February https://lore.kernel.org/all/20210207104022.GA32127@zn.tnic/ (local) 
and some concerns were raised back then around fentry and int3 patching if 
I remember correctly. Is this still an issue?

quoted

IIRC, live-patching used kallsyms/kallsyms_lookup_name for grabbing pointers
to the symbols in the running kernel and then used these pointers to invoke
the functions which reside outside of the live-patch (ie. previously existing
functions). With the above IBT support, if these functions were considered
non-indirectly-reachable, and were suppressed of an endbr, this would lead
into a crash. I remember we were working on klp-convert to fix this through
special relocations and that there were other proposals... but I'm not sure
where it went.

So, would you mind giving a quick update on the general state of this? If the
IBT support would break this (and anything else) regarding live-patching...
and so on?

Right. So, this really depends on how downstream consumers approach this. 
kpatch-build should be fine if I am not mistaken, because it uses the 
.klp.rela support we have in the kernel. We (at SUSE) have a problem, 
because we still exploit kallsyms/kallsyms_lookup_name to cope with this.

Hi Miroslav, Joao,

Yep, kpatch-build uses its own klp-relocation conversion and not kallsyms.

I'm not familiar with CET/IBT, but it sounds like if a function pointer
is not taken at build time (or maybe some other annotation), the
compiler won't generate the needed endbr landing spot in said function?
 And that would be a problem for modules using kallsyms lookup to get to
said function.

Joe, what is the current state of klp-convert? Do we still want to follow 
that way?

kpatch-build relies on klp-relocations and they work well enough across
the architectures we support.  Its code is slightly different than
klp-convert, but concepts the same.  Moving all of that into the kernel
build would definitely be better from a maintenance POV.

I can rebase the latest klp-convert patchset that I had worked on if we
want to test and review.  Apologies for losing momentum on that patchset
while waiting for .klp.arch stuff to drop out, in particular the subset
of static key relocations would still be supported.

-- 
Joe