Re: CET/IBT support and live-patches

Re: CET/IBT support and live-patches

[Miroslav Benes]

[ adding more CCs ]

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

> Hi Miroslav, Petr and Nicolai,
> 
> Long time no talk, I hope you are all still doing great :)

Everything great here :)

> 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/

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

> 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.

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

There was an idea a long time ago to actually rewrite all the relocations 
in a live patch module, so that they are relative to a well defined symbol 
(both in vmlinux and in modules. It would have to be created, I guess.). 
It would be easier tooling-wise and kernel module loader would process it 
like everything else. However, FGKASLR with all its reshuffling would 
render it useless. So something like klp-convert is needed.

Thanks

Miroslav

Re: CET/IBT support and live-patches

[Peter Zijlstra]

On Tue, Nov 23, 2021 at 10:58:57AM +0100, Miroslav Benes wrote:
> [ adding more CCs ]
> 
> On Mon, 22 Nov 2021, joao@overdrivepizza.com wrote:
> 
> > Hi Miroslav, Petr and Nicolai,
> > 
> > Long time no talk, I hope you are all still doing great :)
> 
> Everything great here :)
> 
> > 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/
> 
> Yes, I noticed something was happening again. There was a thread on this 
> in February https://lore.kernel.org/all/20210207104022.GA32127@zn.tnic/ 
> and some concerns were raised back then around fentry and int3 patching if 
> I remember correctly. Is this still an issue?

The problem was bpf, and probably still is. I've not come around to
looking at it. Asusming fentry is at +0 is silly (although I would like
to see that restored for other reasons), but bpf will also need to emit
ENDBR at least at the start of every JIT'ed program, because entry into
them is through an indirect branch.

If nobody beats me to it, I'll get around to it eventually.

Re: CET/IBT support and live-patches

[Miroslav Benes]

On Tue, 23 Nov 2021, Peter Zijlstra wrote:

> On Tue, Nov 23, 2021 at 10:58:57AM +0100, Miroslav Benes wrote:
> > [ adding more CCs ]
> > 
> > On Mon, 22 Nov 2021, joao@overdrivepizza.com wrote:
> > 
> > > Hi Miroslav, Petr and Nicolai,
> > > 
> > > Long time no talk, I hope you are all still doing great :)
> > 
> > Everything great here :)
> > 
> > > 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/
> > 
> > Yes, I noticed something was happening again. There was a thread on this 
> > in February https://lore.kernel.org/all/20210207104022.GA32127@zn.tnic/ 
> > and some concerns were raised back then around fentry and int3 patching if 
> > I remember correctly. Is this still an issue?
> 
> The problem was bpf, and probably still is. I've not come around to
> looking at it. Asusming fentry is at +0 is silly (although I would like
> to see that restored for other reasons), but bpf will also need to emit
> ENDBR at least at the start of every JIT'ed program, because entry into
> them is through an indirect branch.
> 
> If nobody beats me to it, I'll get around to it eventually.

Ok. And we would need something like the following for the livepatch (not 
even compile tested).

---

diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
index 4fe018cc207b..7b9dcd51af32 100644
--- a/arch/powerpc/include/asm/livepatch.h
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -19,16 +19,6 @@ static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
 	regs_set_return_ip(regs, ip);
 }
 
-#define klp_get_ftrace_location klp_get_ftrace_location
-static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
-{
-	/*
-	 * Live patch works only with -mprofile-kernel on PPC. In this case,
-	 * the ftrace location is always within the first 16 bytes.
-	 */
-	return ftrace_location_range(faddr, faddr + 16);
-}
-
 static inline void klp_init_thread_info(struct task_struct *p)
 {
 	/* + 1 to account for STACK_END_MAGIC */
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index fe316c021d73..81cd9235e160 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -127,15 +127,18 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 /*
  * Convert a function address into the appropriate ftrace location.
  *
- * Usually this is just the address of the function, but on some architectures
- * it's more complicated so allow them to provide a custom behaviour.
+ * Usually this is just the address of the function, but there are some
+ * exceptions.
+ *
+ *   * PPC - live patch works only with -mprofile-kernel. In this case,
+ *     the ftrace location is always within the first 16 bytes.
+ *   * x86_64 with CET/IBT enabled - there is ENDBR instruction at +0 offset.
+ *     __fentry__ follows it.
  */
-#ifndef klp_get_ftrace_location
-static unsigned long klp_get_ftrace_location(unsigned long faddr)
+static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
 {
-	return faddr;
+	return ftrace_location_range(faddr, faddr + 16);
 }
-#endif
 
 static void klp_unpatch_func(struct klp_func *func)
 {
 

Re: CET/IBT support and live-patches

[Peter Zijlstra]

On Tue, Nov 23, 2021 at 12:39:15PM +0100, Miroslav Benes wrote:

> Ok. And we would need something like the following for the livepatch (not 
> even compile tested).
> 
> ---
> 
> diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
> index 4fe018cc207b..7b9dcd51af32 100644
> --- a/arch/powerpc/include/asm/livepatch.h
> +++ b/arch/powerpc/include/asm/livepatch.h
> @@ -19,16 +19,6 @@ static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
>  	regs_set_return_ip(regs, ip);
>  }
>  
> -#define klp_get_ftrace_location klp_get_ftrace_location
> -static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
> -{
> -	/*
> -	 * Live patch works only with -mprofile-kernel on PPC. In this case,
> -	 * the ftrace location is always within the first 16 bytes.
> -	 */
> -	return ftrace_location_range(faddr, faddr + 16);
> -}
> -
>  static inline void klp_init_thread_info(struct task_struct *p)
>  {
>  	/* + 1 to account for STACK_END_MAGIC */
> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> index fe316c021d73..81cd9235e160 100644
> --- a/kernel/livepatch/patch.c
> +++ b/kernel/livepatch/patch.c
> @@ -127,15 +127,18 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>  /*
>   * Convert a function address into the appropriate ftrace location.
>   *
> - * Usually this is just the address of the function, but on some architectures
> - * it's more complicated so allow them to provide a custom behaviour.
> + * Usually this is just the address of the function, but there are some
> + * exceptions.
> + *
> + *   * PPC - live patch works only with -mprofile-kernel. In this case,
> + *     the ftrace location is always within the first 16 bytes.
> + *   * x86_64 with CET/IBT enabled - there is ENDBR instruction at +0 offset.
> + *     __fentry__ follows it.
>   */
> -#ifndef klp_get_ftrace_location
> -static unsigned long klp_get_ftrace_location(unsigned long faddr)
> +static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
>  {
> -	return faddr;
> +	return ftrace_location_range(faddr, faddr + 16);
>  }
> -#endif

Agreed, in fact, it should have called at least ftrace_location()
before, as a sanity check the address is in fact a listed fentry site.

I wonder though, given ftrace_cmp_recs() what the behaviour is if
there's two fentry sites within those 16 bytes... I don't think it will
uniquely return the leftmost one, so that might need some thinking.

Consider:

foo:
	endbr
	call __fentry__
	ret;
bar:
	endbr
	call __fentry__
	...

then both sites are within 16 bytes of one another.

Re: CET/IBT support and live-patches

[Steven Rostedt]

On Tue, 23 Nov 2021 12:39:15 +0100 (CET)
Miroslav Benes <mbenes@suse.cz> wrote:

> +++ b/kernel/livepatch/patch.c
> @@ -127,15 +127,18 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>  /*
>   * Convert a function address into the appropriate ftrace location.
>   *
> - * Usually this is just the address of the function, but on some architectures
> - * it's more complicated so allow them to provide a custom behaviour.
> + * Usually this is just the address of the function, but there are some
> + * exceptions.
> + *
> + *   * PPC - live patch works only with -mprofile-kernel. In this case,
> + *     the ftrace location is always within the first 16 bytes.
> + *   * x86_64 with CET/IBT enabled - there is ENDBR instruction at +0 offset.
> + *     __fentry__ follows it.
>   */
> -#ifndef klp_get_ftrace_location
> -static unsigned long klp_get_ftrace_location(unsigned long faddr)
> +static inline unsigned long klp_get_ftrace_location(unsigned long faddr)

Why make this the default function? It should only do this for powerpc and
x86 *if* CET/IBT is enabled.

-- Steve


>  {
> -	return faddr;
> +	return ftrace_location_range(faddr, faddr + 16);
>  }
> -#endif
>  

Re: CET/IBT support and live-patches

[Peter Zijlstra]

On Tue, Nov 23, 2021 at 11:03:20AM -0500, Steven Rostedt wrote:
> On Tue, 23 Nov 2021 12:39:15 +0100 (CET)
> Miroslav Benes <mbenes@suse.cz> wrote:
> 
> > +++ b/kernel/livepatch/patch.c
> > @@ -127,15 +127,18 @@ static void notrace klp_ftrace_handler(unsigned long ip,
> >  /*
> >   * Convert a function address into the appropriate ftrace location.
> >   *
> > - * Usually this is just the address of the function, but on some architectures
> > - * it's more complicated so allow them to provide a custom behaviour.
> > + * Usually this is just the address of the function, but there are some
> > + * exceptions.
> > + *
> > + *   * PPC - live patch works only with -mprofile-kernel. In this case,
> > + *     the ftrace location is always within the first 16 bytes.
> > + *   * x86_64 with CET/IBT enabled - there is ENDBR instruction at +0 offset.
> > + *     __fentry__ follows it.
> >   */
> > -#ifndef klp_get_ftrace_location
> > -static unsigned long klp_get_ftrace_location(unsigned long faddr)
> > +static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
> 
> Why make this the default function? It should only do this for powerpc and
> x86 *if* CET/IBT is enabled.

Well, only this variant of IBT. Once Joao gets his clang patches
together we'll probably have it back at +0.

Something like the below would be more robust, it also gets us something
grep-able for when the IBT code-gen changes yet again.

diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
index 7c5cc6660e4b..4e683a1aa411 100644
--- a/arch/x86/include/asm/livepatch.h
+++ b/arch/x86/include/asm/livepatch.h
@@ -17,4 +17,13 @@ static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
 	ftrace_instruction_pointer_set(fregs, ip);
 }
 
+#define klp_get_ftrace_location klp_get_ftrace_location
+static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
+{
+	unsigned long addr = faddr_location(faddr);
+	if (!addr && IS_ENABLED(CONFIG_X86_IBT))
+		addr = faddr_location(faddr + 4);
+	return addr;
+}
+
 #endif /* _ASM_X86_LIVEPATCH_H */
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index fe316c021d73..fd295bbbcbc7 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -133,7 +133,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 #ifndef klp_get_ftrace_location
 static unsigned long klp_get_ftrace_location(unsigned long faddr)
 {
-	return faddr;
+	return ftrace_location(faddr);
 }
 #endif
 

Re: CET/IBT support and live-patches

[Joe Lawrence]

On 11/23/21 4:58 AM, Miroslav Benes wrote:
> [ adding more CCs ]
> 
> On Mon, 22 Nov 2021, joao@overdrivepizza.com wrote:
> 
>> Hi Miroslav, Petr and Nicolai,
>>
>> Long time no talk, I hope you are all still doing great :)
> 
> Everything great here :)
> 
>> 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/
> 
> Yes, I noticed something was happening again. There was a thread on this 
> in February https://lore.kernel.org/all/20210207104022.GA32127@zn.tnic/ 
> and some concerns were raised back then around fentry and int3 patching if 
> I remember correctly. Is this still an issue?
> 
>> 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

Re: CET/IBT support and live-patches

[Peter Zijlstra]

On Tue, Nov 23, 2021 at 03:58:51PM -0500, Joe Lawrence wrote:

> 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?

Currently it does, but then I'm having objtool scribble it on purpose.

>  And that would be a problem for modules using kallsyms lookup to get to
> said function.

Which is ofcourse the whole purpose of the exercise. If it's not
exported you don't get to call it via a back-door :-) This should kill a
whole heap of dodgy modules quite dead I hope.

Re: CET/IBT support and live-patches

[Miroslav Benes]

On Tue, 23 Nov 2021, Peter Zijlstra wrote:

> On Tue, Nov 23, 2021 at 11:03:20AM -0500, Steven Rostedt wrote:
> > On Tue, 23 Nov 2021 12:39:15 +0100 (CET)
> > Miroslav Benes <mbenes@suse.cz> wrote:
> > 
> > > +++ b/kernel/livepatch/patch.c
> > > @@ -127,15 +127,18 @@ static void notrace klp_ftrace_handler(unsigned long ip,
> > >  /*
> > >   * Convert a function address into the appropriate ftrace location.
> > >   *
> > > - * Usually this is just the address of the function, but on some architectures
> > > - * it's more complicated so allow them to provide a custom behaviour.
> > > + * Usually this is just the address of the function, but there are some
> > > + * exceptions.
> > > + *
> > > + *   * PPC - live patch works only with -mprofile-kernel. In this case,
> > > + *     the ftrace location is always within the first 16 bytes.
> > > + *   * x86_64 with CET/IBT enabled - there is ENDBR instruction at +0 offset.
> > > + *     __fentry__ follows it.
> > >   */
> > > -#ifndef klp_get_ftrace_location
> > > -static unsigned long klp_get_ftrace_location(unsigned long faddr)
> > > +static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
> > 
> > Why make this the default function? It should only do this for powerpc and
> > x86 *if* CET/IBT is enabled.

I thought that given it was a slow path anyway, it would be just nice to 
get rid of the special casing. We support only x86, powerpc and s390x 
after all.

And as Peter later pointed out, it would have the benefit to have a check 
even for default version.
 
> Well, only this variant of IBT. Once Joao gets his clang patches
> together we'll probably have it back at +0.
> 
> Something like the below would be more robust, it also gets us something
> grep-able for when the IBT code-gen changes yet again.
> 
> diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
> index 7c5cc6660e4b..4e683a1aa411 100644
> --- a/arch/x86/include/asm/livepatch.h
> +++ b/arch/x86/include/asm/livepatch.h
> @@ -17,4 +17,13 @@ static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
>  	ftrace_instruction_pointer_set(fregs, ip);
>  }
>  
> +#define klp_get_ftrace_location klp_get_ftrace_location
> +static inline unsigned long klp_get_ftrace_location(unsigned long faddr)
> +{
> +	unsigned long addr = faddr_location(faddr);
> +	if (!addr && IS_ENABLED(CONFIG_X86_IBT))
> +		addr = faddr_location(faddr + 4);
> +	return addr;
> +}
> +
>  #endif /* _ASM_X86_LIVEPATCH_H */
> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> index fe316c021d73..fd295bbbcbc7 100644
> --- a/kernel/livepatch/patch.c
> +++ b/kernel/livepatch/patch.c
> @@ -133,7 +133,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>  #ifndef klp_get_ftrace_location
>  static unsigned long klp_get_ftrace_location(unsigned long faddr)
>  {
> -	return faddr;
> +	return ftrace_location(faddr);
>  }
>  #endif

Yes, this looks nice.

Thanks

Miroslav

Re: CET/IBT support and live-patches

[Miroslav Benes]

Hi,

> > 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.

Ok, then it makes sense to me to pursue this.

> I can rebase the latest klp-convert patchset that I had worked on if we
> want to test and review.

Yes, please. I think we should take a fresher look and start working on it 
(again).

> 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.

No need to apologize. Thank you for maintaining the patch set.

Miroslav

Re: CET/IBT support and live-patches

[Joe Lawrence]

On 11/23/21 4:16 PM, Peter Zijlstra wrote:
> On Tue, Nov 23, 2021 at 03:58:51PM -0500, Joe Lawrence wrote:
> 
>> 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?
> 
> Currently it does, but then I'm having objtool scribble it on purpose.
> 

Hi Peter -- to follow up on the objtool part: what criteria is used to
determine that it may scribble out the endbr?

Thanks,
-- 
Joe

Re: CET/IBT support and live-patches

[Josh Poimboeuf]

On Wed, Dec 01, 2021 at 01:57:00PM -0500, Joe Lawrence wrote:
> On 11/23/21 4:16 PM, Peter Zijlstra wrote:
> > On Tue, Nov 23, 2021 at 03:58:51PM -0500, Joe Lawrence wrote:
> > 
> >> 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?
> > 
> > Currently it does, but then I'm having objtool scribble it on purpose.
> > 
> 
> Hi Peter -- to follow up on the objtool part: what criteria is used to
> determine that it may scribble out the endbr?

ENDBR is "scribbled" for any function for which no function pointer data
relocation exists at vmlinux or module link time.

https://lkml.kernel.org/r/20211122170301.764232470@infradead.org

-- 
Josh