| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220713235556.umnau6nd7u6bz72m@treble>
Date: Wed, 13 Jul 2022 16:55:56 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
kernel test robot <oliver.sang@...el.com>, x86@...nel.org,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] x86: Allow for exclusions in checking RETHUNK
On Wed, Jul 13, 2022 at 02:31:33PM -0700, Kees Cook wrote:
> +++ b/drivers/misc/lkdtm/rodata.c
> @@ -4,8 +4,12 @@
> * (via objcopy tricks), to validate the non-executability of .rodata.
> */
> #include "lkdtm.h"
> +#include <linux/objtool.h>
>
> void noinstr lkdtm_rodata_do_nothing(void)
> {
> /* Does nothing. We just want an architecture agnostic "return". */
> }
Since the function only consists of a single RET instruction, could we
just do an asm(ANNOTATE_UNSAFE_RET) here? i.e. see patch below.
> +/* This is a lie, but given the objcopy, we need objtool to ignore it. */
> +STACK_FRAME_NON_STANDARD(lkdtm_rodata_do_nothing);
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index b341f8a8c7c5..c1b58a682ace 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -902,6 +902,8 @@ static void add_ignores(struct objtool_file *file)
> struct reloc *reloc;
>
> sec = find_section_by_name(file->elf, ".rela.discard.func_stack_frame_non_standard");
> + if (!sec)
> + sec = find_section_by_name(file->elf, ".discard.func_stack_frame_non_standard");
> if (!sec)
> return;
Why was there no .rela section?
Anyway I don't see how this can work, since the code below it traverses
sec->reloc_list, which only exists for rela sections.
Here's the ANNOTATE_UNSAFE_RET idea. Most of the diff is moving the
annotation macros to objtool.h (where they belong anyway).
If there are no objections I can split this up into proper patches
tomorrow.
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index bb05ed4f46bd..9b7cfc68767b 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -63,35 +63,6 @@
#ifdef __ASSEMBLY__
-/*
- * This should be used immediately before an indirect jump/call. It tells
- * objtool the subsequent indirect jump/call is vouched safe for retpoline
- * builds.
- */
-.macro ANNOTATE_RETPOLINE_SAFE
- .Lannotate_\@:
- .pushsection .discard.retpoline_safe
- _ASM_PTR .Lannotate_\@
- .popsection
-.endm
-
-/*
- * (ab)use RETPOLINE_SAFE on RET to annotate away 'bare' RET instructions
- * vs RETBleed validation.
- */
-#define ANNOTATE_UNRET_SAFE ANNOTATE_RETPOLINE_SAFE
-
-/*
- * Abuse ANNOTATE_RETPOLINE_SAFE on a NOP to indicate UNRET_END, should
- * eventually turn into it's own annotation.
- */
-.macro ANNOTATE_UNRET_END
-#ifdef CONFIG_DEBUG_ENTRY
- ANNOTATE_RETPOLINE_SAFE
- nop
-#endif
-.endm
-
/*
* JMP_NOSPEC and CALL_NOSPEC macros can be used instead of a simple
* indirect jmp/call which may be susceptible to the Spectre variant 2
@@ -155,12 +126,6 @@
#else /* __ASSEMBLY__ */
-#define ANNOTATE_RETPOLINE_SAFE \
- "999:\n\t" \
- ".pushsection .discard.retpoline_safe\n\t" \
- _ASM_PTR " 999b\n\t" \
- ".popsection\n\t"
-
typedef u8 retpoline_thunk_t[RETPOLINE_THUNK_SIZE];
extern retpoline_thunk_t __x86_indirect_thunk_array[];
diff --git a/drivers/misc/lkdtm/rodata.c b/drivers/misc/lkdtm/rodata.c
index 708a2558a7ac..b4aeb50c4a96 100644
--- a/drivers/misc/lkdtm/rodata.c
+++ b/drivers/misc/lkdtm/rodata.c
@@ -8,6 +8,7 @@
void noinstr lkdtm_rodata_do_nothing(void)
{
+ asm(ANNOTATE_RETPOLINE_SAFE);
/* Does nothing. We just want an architecture agnostic "return". */
}
diff --git a/include/linux/objtool.h b/include/linux/objtool.h
index 10bc88cc3bf6..0bd80ba8e6b2 100644
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -43,6 +43,12 @@ struct unwind_hint {
#define UNWIND_HINT_TYPE_SAVE 5
#define UNWIND_HINT_TYPE_RESTORE 6
+/*
+ * (ab)use RETPOLINE_SAFE on RET to annotate away 'bare' RET instructions
+ * vs RETBleed validation.
+ */
+#define ANNOTATE_UNRET_SAFE ANNOTATE_RETPOLINE_SAFE
+
#ifdef CONFIG_OBJTOOL
#include <asm/asm.h>
@@ -84,6 +90,12 @@ struct unwind_hint {
#define STACK_FRAME_NON_STANDARD_FP(func)
#endif
+#define ANNOTATE_RETPOLINE_SAFE \
+ "999:\n\t" \
+ ".pushsection .discard.retpoline_safe\n\t" \
+ _ASM_PTR " 999b\n\t" \
+ ".popsection\n\t"
+
#define ANNOTATE_NOENDBR \
"986: \n\t" \
".pushsection .discard.noendbr\n\t" \
@@ -98,6 +110,29 @@ struct unwind_hint {
#else /* __ASSEMBLY__ */
+/*
+ * This should be used immediately before an indirect jump/call. It tells
+ * objtool the subsequent indirect jump/call is vouched safe for retpoline
+ * builds.
+ */
+.macro ANNOTATE_RETPOLINE_SAFE
+ .Lannotate_\@:
+ .pushsection .discard.retpoline_safe
+ _ASM_PTR .Lannotate_\@
+ .popsection
+.endm
+
+/*
+ * Abuse ANNOTATE_RETPOLINE_SAFE on a NOP to indicate UNRET_END, should
+ * eventually turn into it's own annotation.
+ */
+.macro ANNOTATE_UNRET_END
+#ifdef CONFIG_DEBUG_ENTRY
+ ANNOTATE_RETPOLINE_SAFE
+ nop
+#endif
+.endm
+
/*
* This macro indicates that the following intra-function call is valid.
* Any non-annotated intra-function call will cause objtool to issue a warning.
@@ -172,6 +207,8 @@ struct unwind_hint {
#else /* !CONFIG_OBJTOOL */
+#define ANNOTATE_RETPOLINE_SAFE
+
#ifndef __ASSEMBLY__
#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
diff --git a/tools/include/linux/objtool.h b/tools/include/linux/objtool.h
index 10bc88cc3bf6..0bd80ba8e6b2 100644
--- a/tools/include/linux/objtool.h
+++ b/tools/include/linux/objtool.h
@@ -43,6 +43,12 @@ struct unwind_hint {
#define UNWIND_HINT_TYPE_SAVE 5
#define UNWIND_HINT_TYPE_RESTORE 6
+/*
+ * (ab)use RETPOLINE_SAFE on RET to annotate away 'bare' RET instructions
+ * vs RETBleed validation.
+ */
+#define ANNOTATE_UNRET_SAFE ANNOTATE_RETPOLINE_SAFE
+
#ifdef CONFIG_OBJTOOL
#include <asm/asm.h>
@@ -84,6 +90,12 @@ struct unwind_hint {
#define STACK_FRAME_NON_STANDARD_FP(func)
#endif
+#define ANNOTATE_RETPOLINE_SAFE \
+ "999:\n\t" \
+ ".pushsection .discard.retpoline_safe\n\t" \
+ _ASM_PTR " 999b\n\t" \
+ ".popsection\n\t"
+
#define ANNOTATE_NOENDBR \
"986: \n\t" \
".pushsection .discard.noendbr\n\t" \
@@ -98,6 +110,29 @@ struct unwind_hint {
#else /* __ASSEMBLY__ */
+/*
+ * This should be used immediately before an indirect jump/call. It tells
+ * objtool the subsequent indirect jump/call is vouched safe for retpoline
+ * builds.
+ */
+.macro ANNOTATE_RETPOLINE_SAFE
+ .Lannotate_\@:
+ .pushsection .discard.retpoline_safe
+ _ASM_PTR .Lannotate_\@
+ .popsection
+.endm
+
+/*
+ * Abuse ANNOTATE_RETPOLINE_SAFE on a NOP to indicate UNRET_END, should
+ * eventually turn into it's own annotation.
+ */
+.macro ANNOTATE_UNRET_END
+#ifdef CONFIG_DEBUG_ENTRY
+ ANNOTATE_RETPOLINE_SAFE
+ nop
+#endif
+.endm
+
/*
* This macro indicates that the following intra-function call is valid.
* Any non-annotated intra-function call will cause objtool to issue a warning.
@@ -172,6 +207,8 @@ struct unwind_hint {
#else /* !CONFIG_OBJTOOL */
+#define ANNOTATE_RETPOLINE_SAFE
+
#ifndef __ASSEMBLY__
#define UNWIND_HINT(sp_reg, sp_offset, type, end) \
Powered by blists - more mailing lists