From: Lai Jiangshan <[email protected]>
Some entry code will be implemented in C files.
Introduce __entry_text to set them in .entry.text section.
The new __entry_text disables instrumentation like noinstr, so
__noinstr_section() is added for noinstr and the new __entry_text.
Note, entry code can not access to %gs before the %gs base is switched
to kernel %gs base, so stack protector can not be used on the C entry
code. But __entry_text doesn't disable stack protector since some
compilers might not support function level granular attribute to
disable stack protector. It will be disabled in C file level.
Cc: Borislav Petkov <[email protected]>
Reviewed-by: Miguel Ojeda <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Suggested-by: Nick Desaulniers <[email protected]>
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Lai Jiangshan <[email protected]>
---
arch/x86/include/asm/idtentry.h | 3 +++
include/linux/compiler_types.h | 8 +++++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 72184b0b2219..acc4c99f801c 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -13,6 +13,9 @@
#include <asm/irq_stack.h>
+/* Entry code written in C. */
+#define __entry_text __noinstr_section(".entry.text")
+
/**
* DECLARE_IDTENTRY - Declare functions for simple IDT entry points
* No error code pushed by hardware
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1c2c33ae1b37..8c7e81efe9bf 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -225,9 +225,11 @@ struct ftrace_likely_data {
#endif
/* Section for code which can't be instrumented at all */
-#define noinstr \
- noinline notrace __attribute((__section__(".noinstr.text"))) \
- __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage
+#define __noinstr_section(section) \
+ noinline notrace __section(section) __no_profile \
+ __no_kcsan __no_sanitize_address __no_sanitize_coverage
+
+#define noinstr __noinstr_section(".noinstr.text")
#endif /* __KERNEL__ */
--
2.19.1.6.gb485710b
On Wed, May 11, 2022 at 9:34 PM Nick Desaulniers
<[email protected]> wrote:
>
> Yeah, nvm, I missed the bottom #define for some reason.
No worries, I got worried there could be a change of behavior I did
not know about the order of those... (in which case we should really
document it! :)
Cheers,
Miguel
On Wed, May 11 2022 at 11:01, Nick Desaulniers wrote:
> On Wed, May 11, 2022 at 12:27 AM Lai Jiangshan <[email protected]> wrote:
>> /* Section for code which can't be instrumented at all */
>> -#define noinstr \
>> - noinline notrace __attribute((__section__(".noinstr.text"))) \
>> - __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage
>> +#define __noinstr_section(section) \
>> + noinline notrace __section(section) __no_profile \
>> + __no_kcsan __no_sanitize_address __no_sanitize_coverage
>> +
>> +#define noinstr __noinstr_section(".noinstr.text")
>
> I haven't looked at the rest of the series, but isn't `noinstr` used
> in a bunch of places? Wont this commit break all those uses or at
> least make it break bisection?
Why so?
This is still applying all the 'no' annotations and puts the code in
the very same section as before, no?
Thanks,
tglx
On Wed, May 11, 2022 at 12:27 PM Miguel Ojeda
<[email protected]> wrote:
>
> On Wed, May 11, 2022 at 8:01 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > I haven't looked at the rest of the series, but isn't `noinstr` used
> > in a bunch of places? Wont this commit break all those uses or at
>
> Except the order, it expands to the same, no? Or what is the issue?
Yeah, nvm, I missed the bottom #define for some reason.
--
Thanks,
~Nick Desaulniers
On Wed, May 11, 2022 at 8:01 PM Nick Desaulniers
<[email protected]> wrote:
>
> I haven't looked at the rest of the series, but isn't `noinstr` used
> in a bunch of places? Wont this commit break all those uses or at
Except the order, it expands to the same, no? Or what is the issue?
Cheers,
Miguel
On Wed, May 11, 2022 at 12:27 AM Lai Jiangshan <[email protected]> wrote:
>
> From: Lai Jiangshan <[email protected]>
>
> Some entry code will be implemented in C files.
> Introduce __entry_text to set them in .entry.text section.
>
> The new __entry_text disables instrumentation like noinstr, so
> __noinstr_section() is added for noinstr and the new __entry_text.
>
> Note, entry code can not access to %gs before the %gs base is switched
> to kernel %gs base, so stack protector can not be used on the C entry
> code. But __entry_text doesn't disable stack protector since some
> compilers might not support function level granular attribute to
> disable stack protector. It will be disabled in C file level.
>
> Cc: Borislav Petkov <[email protected]>
> Reviewed-by: Miguel Ojeda <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Suggested-by: Nick Desaulniers <[email protected]>
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Lai Jiangshan <[email protected]>
> ---
> arch/x86/include/asm/idtentry.h | 3 +++
> include/linux/compiler_types.h | 8 +++++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
> index 72184b0b2219..acc4c99f801c 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -13,6 +13,9 @@
>
> #include <asm/irq_stack.h>
>
> +/* Entry code written in C. */
> +#define __entry_text __noinstr_section(".entry.text")
> +
> /**
> * DECLARE_IDTENTRY - Declare functions for simple IDT entry points
> * No error code pushed by hardware
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 1c2c33ae1b37..8c7e81efe9bf 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -225,9 +225,11 @@ struct ftrace_likely_data {
> #endif
>
> /* Section for code which can't be instrumented at all */
> -#define noinstr \
> - noinline notrace __attribute((__section__(".noinstr.text"))) \
> - __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage
> +#define __noinstr_section(section) \
> + noinline notrace __section(section) __no_profile \
> + __no_kcsan __no_sanitize_address __no_sanitize_coverage
> +
> +#define noinstr __noinstr_section(".noinstr.text")
I haven't looked at the rest of the series, but isn't `noinstr` used
in a bunch of places? Wont this commit break all those uses or at
least make it break bisection?
Also, my suggestion was simply not to open code
__attribute__((section(""))). Does this work?
```
diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 7924f27f5c8b..10ec7039e17d 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -13,6 +13,9 @@
#include <asm/irq_stack.h>
+/* Entry code written in C. Override the section used by noinstr. */
+#define __entry_text noinstr __section(".entry.text")
+
/**
* DECLARE_IDTENTRY - Declare functions for simple IDT entry points
* No error code pushed by hardware
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1c2c33ae1b37..ce623099eb21 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -226,8 +226,8 @@ struct ftrace_likely_data {
/* Section for code which can't be instrumented at all */
#define noinstr
\
- noinline notrace __attribute((__section__(".noinstr.text"))) \
- __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage
+ noinline notrace __section(".noinstr.text") __no_kcsan \
+ __no_sanitize_address __no_profile __no_sanitize_coverage
#endif /* __KERNEL__ */
```
--
Thanks,
~Nick Desaulniers