* [PATCH 0/3] objtool: noinstr validation for static branches/calls
@ 2024-11-23 1:52 Josh Poimboeuf
2024-11-23 1:52 ` [PATCH 1/3] jump_label: Add annotations for validating noinstr usage Josh Poimboeuf
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Josh Poimboeuf @ 2024-11-23 1:52 UTC (permalink / raw)
To: Valentin Schneider; +Cc: linux-kernel, Peter Zijlstra
Valentin, feel free to include in your next version.
Josh Poimboeuf (3):
jump_label: Add annotations for validating noinstr usage
static_call: Add read-only-after-init static calls
objtool: Add noinstr validation for static branches/calls
include/linux/jump_label.h | 29 ++++++--
include/linux/objtool.h | 6 ++
include/linux/static_call.h | 18 +++++
tools/objtool/Documentation/objtool.txt | 34 +++++++++
tools/objtool/check.c | 92 ++++++++++++++++++++++---
tools/objtool/include/objtool/check.h | 1 +
tools/objtool/include/objtool/elf.h | 1 +
tools/objtool/include/objtool/special.h | 1 +
tools/objtool/special.c | 20 +++++-
9 files changed, 186 insertions(+), 16 deletions(-)
--
2.47.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] jump_label: Add annotations for validating noinstr usage 2024-11-23 1:52 [PATCH 0/3] objtool: noinstr validation for static branches/calls Josh Poimboeuf @ 2024-11-23 1:52 ` Josh Poimboeuf 2024-11-23 1:54 ` Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 2/3] static_call: Add read-only-after-init static calls Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf 2 siblings, 1 reply; 8+ messages in thread From: Josh Poimboeuf @ 2024-11-23 1:52 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, Peter Zijlstra Deferring a code patching IPI is unsafe if the patched code is in a noinstr region. In that case the text poke code must trigger an immediate IPI to all CPUs, which can rudely interrupt an isolated NO_HZ CPU running in userspace. Some noinstr static branches may really need to be patched at runtime, despite the resulting disruption. Add DEFINE_STATIC_KEY_*_NOINSTR() variants for those. They don't do anything special yet; that will come later. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- include/linux/jump_label.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h index f5a2727ca4a9..88bb6e32fdcb 100644 --- a/include/linux/jump_label.h +++ b/include/linux/jump_label.h @@ -385,6 +385,23 @@ struct static_key_false { #define DEFINE_STATIC_KEY_FALSE_RO(name) \ struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT +/* + * The _NOINSTR variants are used to tell objtool the static key is allowed to + * be used in noinstr code. + * + * They should almost never be used, as they prevent code patching IPIs from + * being deferred, which can be problematic for isolated NOHZ_FULL CPUs running + * in pure userspace. + * + * If using one of these _NOINSTR variants, please add a comment above the + * definition with the rationale. + */ +#define DEFINE_STATIC_KEY_TRUE_NOINSTR(name) \ + DEFINE_STATIC_KEY_TRUE(name) + +#define DEFINE_STATIC_KEY_FALSE_NOINSTR(name) \ + DEFINE_STATIC_KEY_FALSE(name) + #define DECLARE_STATIC_KEY_FALSE(name) \ extern struct static_key_false name -- 2.47.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] jump_label: Add annotations for validating noinstr usage 2024-11-23 1:52 ` [PATCH 1/3] jump_label: Add annotations for validating noinstr usage Josh Poimboeuf @ 2024-11-23 1:54 ` Josh Poimboeuf 0 siblings, 0 replies; 8+ messages in thread From: Josh Poimboeuf @ 2024-11-23 1:54 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, Peter Zijlstra On Fri, Nov 22, 2024 at 05:52:20PM -0800, Josh Poimboeuf wrote: > Deferring a code patching IPI is unsafe if the patched code is in a > noinstr region. In that case the text poke code must trigger an > immediate IPI to all CPUs, which can rudely interrupt an isolated NO_HZ > CPU running in userspace. > > Some noinstr static branches may really need to be patched at runtime, > despite the resulting disruption. Add DEFINE_STATIC_KEY_*_NOINSTR() > variants for those. They don't do anything special yet; that will come > later. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Oops, $SUBJECT should probably be changed to something like: jump_label: Add DEFINE_STATIC_KEY_*_NOINSTR() -- Josh ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] static_call: Add read-only-after-init static calls 2024-11-23 1:52 [PATCH 0/3] objtool: noinstr validation for static branches/calls Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 1/3] jump_label: Add annotations for validating noinstr usage Josh Poimboeuf @ 2024-11-23 1:52 ` Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf 2 siblings, 0 replies; 8+ messages in thread From: Josh Poimboeuf @ 2024-11-23 1:52 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, Peter Zijlstra Deferring a code patching IPI is unsafe if the patched code is in a noinstr region. In that case the text poke code must trigger an immediate IPI to all CPUs, which can rudely interrupt an isolated NO_HZ CPU running in userspace. If a noinstr static call only needs to be patched during boot, its key can be made ro-after-init to ensure it will never be patched at runtime. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- include/linux/static_call.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/linux/static_call.h b/include/linux/static_call.h index 141e6b176a1b..34970e178fdf 100644 --- a/include/linux/static_call.h +++ b/include/linux/static_call.h @@ -190,6 +190,14 @@ extern long __static_call_return0(void); }; \ ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func) +#define DEFINE_STATIC_CALL_RO(name, _func) \ + DECLARE_STATIC_CALL(name, _func); \ + struct static_call_key __ro_after_init STATIC_CALL_KEY(name) = {\ + .func = _func, \ + .type = 1, \ + }; \ + ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func) + #define DEFINE_STATIC_CALL_NULL(name, _func) \ DECLARE_STATIC_CALL(name, _func); \ struct static_call_key STATIC_CALL_KEY(name) = { \ @@ -198,6 +206,14 @@ extern long __static_call_return0(void); }; \ ARCH_DEFINE_STATIC_CALL_NULL_TRAMP(name) +#define DEFINE_STATIC_CALL_NULL_RO(name, _func) \ + DECLARE_STATIC_CALL(name, _func); \ + struct static_call_key __ro_after_init STATIC_CALL_KEY(name) = {\ + .func = NULL, \ + .type = 1, \ + }; \ + ARCH_DEFINE_STATIC_CALL_NULL_TRAMP(name) + #define DEFINE_STATIC_CALL_RET0(name, _func) \ DECLARE_STATIC_CALL(name, _func); \ struct static_call_key STATIC_CALL_KEY(name) = { \ -- 2.47.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] objtool: Add noinstr validation for static branches/calls 2024-11-23 1:52 [PATCH 0/3] objtool: noinstr validation for static branches/calls Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 1/3] jump_label: Add annotations for validating noinstr usage Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 2/3] static_call: Add read-only-after-init static calls Josh Poimboeuf @ 2024-11-23 1:52 ` Josh Poimboeuf 2024-11-25 7:08 ` kernel test robot ` (2 more replies) 2 siblings, 3 replies; 8+ messages in thread From: Josh Poimboeuf @ 2024-11-23 1:52 UTC (permalink / raw) To: Valentin Schneider; +Cc: linux-kernel, Peter Zijlstra Warn about static branches/calls in noinstr regions, unless the corresponding key is RO-after-init or has been manually whitelisted with DEFINE_STATIC_KEY_*_NOINSTR((). Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- include/linux/jump_label.h | 16 +++-- include/linux/objtool.h | 6 ++ include/linux/static_call.h | 2 + tools/objtool/Documentation/objtool.txt | 34 +++++++++ tools/objtool/check.c | 92 ++++++++++++++++++++++--- tools/objtool/include/objtool/check.h | 1 + tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/special.h | 1 + tools/objtool/special.c | 20 +++++- 9 files changed, 155 insertions(+), 18 deletions(-) diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h index 88bb6e32fdcb..eb3e118accc4 100644 --- a/include/linux/jump_label.h +++ b/include/linux/jump_label.h @@ -373,8 +373,9 @@ struct static_key_false { #define DEFINE_STATIC_KEY_TRUE(name) \ struct static_key_true name = STATIC_KEY_TRUE_INIT -#define DEFINE_STATIC_KEY_TRUE_RO(name) \ - struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT +#define DEFINE_STATIC_KEY_TRUE_RO(name) \ + struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT; \ + ANNOTATE_NOINSTR_ALLOWED(name) #define DECLARE_STATIC_KEY_TRUE(name) \ extern struct static_key_true name @@ -382,8 +383,9 @@ struct static_key_false { #define DEFINE_STATIC_KEY_FALSE(name) \ struct static_key_false name = STATIC_KEY_FALSE_INIT -#define DEFINE_STATIC_KEY_FALSE_RO(name) \ - struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT +#define DEFINE_STATIC_KEY_FALSE_RO(name) \ + struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ + ANNOTATE_NOINSTR_ALLOWED(name) /* * The _NOINSTR variants are used to tell objtool the static key is allowed to @@ -397,10 +399,12 @@ struct static_key_false { * definition with the rationale. */ #define DEFINE_STATIC_KEY_TRUE_NOINSTR(name) \ - DEFINE_STATIC_KEY_TRUE(name) + DEFINE_STATIC_KEY_TRUE(name); \ + ANNOTATE_NOINSTR_ALLOWED(name) #define DEFINE_STATIC_KEY_FALSE_NOINSTR(name) \ - DEFINE_STATIC_KEY_FALSE(name) + DEFINE_STATIC_KEY_FALSE(name); \ + ANNOTATE_NOINSTR_ALLOWED(name) #define DECLARE_STATIC_KEY_FALSE(name) \ extern struct static_key_false name diff --git a/include/linux/objtool.h b/include/linux/objtool.h index b3b8d3dab52d..53de92666c48 100644 --- a/include/linux/objtool.h +++ b/include/linux/objtool.h @@ -34,6 +34,12 @@ static void __used __section(".discard.func_stack_frame_non_standard") \ *__func_stack_frame_non_standard_##func = func +#define __ANNOTATE_NOINSTR_ALLOWED(key) \ + static void __used __section(".discard.noinstr_allowed") \ + *__annotate_noinstr_allowed_##key = &key + +#define ANNOTATE_NOINSTR_ALLOWED(key) __ANNOTATE_NOINSTR_ALLOWED(key) + /* * STACK_FRAME_NON_STANDARD_FP() is a frame-pointer-specific function ignore * for the case where a function is intentionally missing frame pointer setup, diff --git a/include/linux/static_call.h b/include/linux/static_call.h index 34970e178fdf..a82d0bd7860d 100644 --- a/include/linux/static_call.h +++ b/include/linux/static_call.h @@ -196,6 +196,7 @@ extern long __static_call_return0(void); .func = _func, \ .type = 1, \ }; \ + ANNOTATE_NOINSTR_ALLOWED(STATIC_CALL_TRAMP(name)); \ ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func) #define DEFINE_STATIC_CALL_NULL(name, _func) \ @@ -212,6 +213,7 @@ extern long __static_call_return0(void); .func = NULL, \ .type = 1, \ }; \ + ANNOTATE_NOINSTR_ALLOWED(STATIC_CALL_TRAMP(name)); \ ARCH_DEFINE_STATIC_CALL_NULL_TRAMP(name) #define DEFINE_STATIC_CALL_RET0(name, _func) \ diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt index 7c3ee959b63c..922d3b41541d 100644 --- a/tools/objtool/Documentation/objtool.txt +++ b/tools/objtool/Documentation/objtool.txt @@ -447,6 +447,40 @@ the objtool maintainers. names and does not use module_init() / module_exit() macros to create them. +13. file.o: warning: func()+0x2a: key: non-RO static key usage in noinstr code + file.o: warning: func()+0x2a: key: non-RO static call usage in noinstr code + + This means that noinstr function func() uses a static key or + static call named 'key' which can be modified at runtime. This is + discouraged because it prevents code patching IPIs from being + deferred. + + You have the following options: + + 1) Check whether the static key/call in question is only modified + during init. If so, define it as read-only-after-init with + DEFINE_STATIC_KEY_*_RO() or DEFINE_STATIC_CALL_RO(). + + 2) Avoid the runtime patching. For static keys this can be done by + using static_key_enabled() or by getting rid of the static key + altogether if performance is not a concern. + + For static calls, something like the following could be done: + + target = static_call_query(foo); + if (target == func1) + func1(); + else if (target == func2) + func2(); + ... + + 3) Silence the warning by defining the static key/call with + DEFINE_STATIC_*_NOINSTR(). This decision should not + be taken lightly as it may result in code patching IPIs getting + sent to isolated NOHZ_FULL CPUs running in pure userspace. A + comment should be added above the definition explaining the + rationale for the decision. + If the error doesn't seem to make sense, it could be a bug in objtool. Feel free to ask the objtool maintainer for help. diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7fc96c30b79c..225a075bc79e 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1068,6 +1068,45 @@ static int create_direct_call_sections(struct objtool_file *file) return 0; } +static int read_noinstr_allowed(struct objtool_file *file) +{ + struct section *rsec; + struct symbol *sym; + struct reloc *reloc; + + rsec = find_section_by_name(file->elf, ".rela.discard.noinstr_allowed"); + if (!rsec) + return 0; + + for_each_reloc(rsec, reloc) { + switch (reloc->sym->type) { + case STT_OBJECT: + case STT_FUNC: + sym = reloc->sym; + break; + + case STT_SECTION: + sym = find_symbol_by_offset(reloc->sym->sec, + reloc_addend(reloc)); + if (!sym) { + WARN_FUNC("can't find static key/call symbol", + reloc->sym->sec, reloc_addend(reloc)); + return -1; + } + break; + + default: + WARN("unexpected relocation symbol type in %s: %d", + rsec->name, reloc->sym->type); + return -1; + } + + sym->noinstr_allowed = 1; + } + + return 0; +} + /* * Warnings shouldn't be reported for ignored functions. */ @@ -1955,6 +1994,8 @@ static int handle_jump_alt(struct objtool_file *file, return -1; } + orig_insn->key = special_alt->key; + if (opts.hack_jump_label && special_alt->key_addend & 2) { struct reloc *reloc = insn_reloc(file, orig_insn); @@ -2731,6 +2772,10 @@ static int decode_sections(struct objtool_file *file) if (ret) return ret; + ret = read_noinstr_allowed(file); + if (ret) + return ret; + return 0; } @@ -3494,9 +3539,9 @@ static bool pv_call_dest(struct objtool_file *file, struct instruction *insn) return file->pv_ops[idx].clean; } -static inline bool noinstr_call_dest(struct objtool_file *file, - struct instruction *insn, - struct symbol *func) +static inline bool noinstr_call_allowed(struct objtool_file *file, + struct instruction *insn, + struct symbol *func) { /* * We can't deal with indirect function calls at present; @@ -3516,10 +3561,10 @@ static inline bool noinstr_call_dest(struct objtool_file *file, return true; /* - * If the symbol is a static_call trampoline, we can't tell. + * Only DEFINE_STATIC_CALL_*_RO allowed. */ if (func->static_call_tramp) - return true; + return func->noinstr_allowed; /* * The __ubsan_handle_*() calls are like WARN(), they only happen when @@ -3532,14 +3577,29 @@ static inline bool noinstr_call_dest(struct objtool_file *file, return false; } +static char *static_call_name(struct symbol *func) +{ + return func->name + strlen("__SCT__"); +} + static int validate_call(struct objtool_file *file, struct instruction *insn, struct insn_state *state) { - if (state->noinstr && state->instr <= 0 && - !noinstr_call_dest(file, insn, insn_call_dest(insn))) { - WARN_INSN(insn, "call to %s() leaves .noinstr.text section", call_dest_name(insn)); - return 1; + if (state->noinstr && state->instr <= 0) { + struct symbol *dest = insn_call_dest(insn); + + if (dest->static_call_tramp) { + if (!dest->noinstr_allowed) { + WARN_INSN(insn, "non-RO static call usage for '%s' in noinstr", + static_call_name(dest)); + } + + } else if (!noinstr_call_allowed(file, insn, dest)) { + WARN_INSN(insn, "call to %s() leaves .noinstr.text section", + call_dest_name(insn)); + return 1; + } } if (state->uaccess && !func_uaccess_safe(insn_call_dest(insn))) { @@ -3602,6 +3662,17 @@ static int validate_return(struct symbol *func, struct instruction *insn, struct return 0; } +static int validate_static_key(struct instruction *insn, struct insn_state *state) +{ + if (state->noinstr && state->instr <= 0 && !insn->key->noinstr_allowed) { + WARN_INSN(insn, "non-RO static key usage for '%s' in noinstr", + insn->key->name); + return 1; + } + + return 0; +} + static struct instruction *next_insn_to_validate(struct objtool_file *file, struct instruction *insn) { @@ -3763,6 +3834,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, if (handle_insn_ops(insn, next_insn, &state)) return 1; + if (insn->key) + validate_static_key(insn, &state); + switch (insn->type) { case INSN_RETURN: diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h index daa46f1f0965..c0da7246eac7 100644 --- a/tools/objtool/include/objtool/check.h +++ b/tools/objtool/include/objtool/check.h @@ -77,6 +77,7 @@ struct instruction { struct symbol *sym; struct stack_op *stack_ops; struct cfi_state *cfi; + struct symbol *key; }; static inline struct symbol *insn_func(struct instruction *insn) diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index d7e815c2fd15..0cb79931262b 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -69,6 +69,7 @@ struct symbol { u8 embedded_insn : 1; u8 local_label : 1; u8 frame_pointer : 1; + u8 noinstr_allowed : 1; struct list_head pv_target; struct reloc *relocs; }; diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h index 86d4af9c5aa9..ce4759358ec4 100644 --- a/tools/objtool/include/objtool/special.h +++ b/tools/objtool/include/objtool/special.h @@ -20,6 +20,7 @@ struct special_alt { bool skip_alt; bool jump_or_nop; u8 key_addend; + struct symbol *key; struct section *orig_sec; unsigned long orig_off; diff --git a/tools/objtool/special.c b/tools/objtool/special.c index 097a69db82a0..d0a9cca27a9c 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -119,14 +119,28 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, if (entry->key) { struct reloc *key_reloc; + struct symbol *key; + s64 key_addend; key_reloc = find_reloc_by_dest(elf, sec, offset + entry->key); if (!key_reloc) { - WARN_FUNC("can't find key reloc", - sec, offset + entry->key); + WARN_FUNC("can't find key reloc", sec, offset + entry->key); return -1; } - alt->key_addend = reloc_addend(key_reloc); + + key = key_reloc->sym; + key_addend = reloc_addend(key_reloc); + + if (key->type == STT_SECTION) + key = find_symbol_by_offset(key->sec, key_addend & ~3); + + if (!key) { + WARN_FUNC("can't find key sym", sec, offset + entry->key); + return -1; + } + + alt->key = key; + alt->key_addend = key_addend; } return 0; -- 2.47.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf @ 2024-11-25 7:08 ` kernel test robot 2024-11-25 8:00 ` kernel test robot 2024-11-25 10:47 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2024-11-25 7:08 UTC (permalink / raw) To: Josh Poimboeuf, Valentin Schneider Cc: llvm, oe-kbuild-all, linux-kernel, Peter Zijlstra Hi Josh, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.12 next-20241122] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/jump_label-Add-annotations-for-validating-noinstr-usage/20241125-105905 base: linus/master patch link: https://lore.kernel.org/r/b5746646456eb030673cdb62c23d69ed83c2702a.1732326588.git.jpoimboe%40kernel.org patch subject: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls config: powerpc-motionpro_defconfig (https://download.01.org/0day-ci/archive/20241125/202411251459.P7KctSbw-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411251459.P7KctSbw-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411251459.P7KctSbw-lkp@intel.com/ All errors (new ones prefixed by >>): >> init/main.c:821:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int] 821 | DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, | ^ include/linux/jump_label.h:430:2: note: expanded from macro 'DEFINE_STATIC_KEY_MAYBE_RO' 430 | __PASTE(_DEFINE_STATIC_KEY_RO_, IS_ENABLED(cfg))(name) | ^ include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE' 84 | #define __PASTE(a,b) ___PASTE(a,b) | ^ include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE' 83 | #define ___PASTE(a,b) a##b | ^ <scratch space>:51:1: note: expanded from here 51 | _DEFINE_STATIC_KEY_RO_0 | ^ include/linux/jump_label.h:428:39: note: expanded from macro '_DEFINE_STATIC_KEY_RO_0' 428 | #define _DEFINE_STATIC_KEY_RO_0(name) DEFINE_STATIC_KEY_FALSE_RO(name) | ^ include/linux/jump_label.h:388:2: note: expanded from macro 'DEFINE_STATIC_KEY_FALSE_RO' 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^ >> init/main.c:822:7: error: a parameter list without types is only allowed in a function definition 822 | randomize_kstack_offset); | ^ 2 errors generated. vim +/int +821 init/main.c 4e37958d1288ce Steven Rostedt (VMware 2018-03-26 819) 8cb37a5974a485 Marco Elver 2022-01-31 820 #ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET 39218ff4c625db Kees Cook 2021-04-01 @821 DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, 39218ff4c625db Kees Cook 2021-04-01 @822 randomize_kstack_offset); 39218ff4c625db Kees Cook 2021-04-01 823 DEFINE_PER_CPU(u32, kstack_offset); 39218ff4c625db Kees Cook 2021-04-01 824 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf 2024-11-25 7:08 ` kernel test robot @ 2024-11-25 8:00 ` kernel test robot 2024-11-25 10:47 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2024-11-25 8:00 UTC (permalink / raw) To: Josh Poimboeuf, Valentin Schneider Cc: oe-kbuild-all, linux-kernel, Peter Zijlstra Hi Josh, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.12 next-20241125] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/jump_label-Add-annotations-for-validating-noinstr-usage/20241125-105905 base: linus/master patch link: https://lore.kernel.org/r/b5746646456eb030673cdb62c23d69ed83c2702a.1732326588.git.jpoimboe%40kernel.org patch subject: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls config: m68k-bvme6000_defconfig (https://download.01.org/0day-ci/archive/20241125/202411251530.5P17DWTE-lkp@intel.com/config) compiler: m68k-linux-gcc (GCC) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411251530.5P17DWTE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411251530.5P17DWTE-lkp@intel.com/ All error/warnings (new ones prefixed by >>): In file included from include/linux/static_key.h:1, from include/linux/alloc_tag.h:15, from include/linux/percpu.h:5, from include/linux/percpu_counter.h:14, from include/linux/mm_types.h:21, from include/linux/mmzone.h:22, from include/linux/gfp.h:7, from include/linux/mm.h:7, from mm/usercopy.c:13: >> include/linux/jump_label.h:388:9: warning: data definition has no type or storage class 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ mm/usercopy.c:204:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 204 | static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/jump_label.h:388:9: error: type defaults to 'int' in declaration of 'ANNOTATE_NOINSTR_ALLOWED' [-Wimplicit-int] 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ mm/usercopy.c:204:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 204 | static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/jump_label.h:371:39: error: parameter names (without types) in function declaration [-Wdeclaration-missing-parameter-type] 371 | #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } | ^~~~~~~~~~~~~~~~ include/linux/jump_label.h:387:56: note: in expansion of macro 'STATIC_KEY_FALSE_INIT' 387 | struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ | ^~~~~~~~~~~~~~~~~~~~~ mm/usercopy.c:204:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 204 | static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ vim +388 include/linux/jump_label.h 369 370 #define STATIC_KEY_TRUE_INIT (struct static_key_true) { .key = STATIC_KEY_INIT_TRUE, } > 371 #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } 372 373 #define DEFINE_STATIC_KEY_TRUE(name) \ 374 struct static_key_true name = STATIC_KEY_TRUE_INIT 375 376 #define DEFINE_STATIC_KEY_TRUE_RO(name) \ 377 struct static_key_true name __ro_after_init = STATIC_KEY_TRUE_INIT; \ 378 ANNOTATE_NOINSTR_ALLOWED(name) 379 380 #define DECLARE_STATIC_KEY_TRUE(name) \ 381 extern struct static_key_true name 382 383 #define DEFINE_STATIC_KEY_FALSE(name) \ 384 struct static_key_false name = STATIC_KEY_FALSE_INIT 385 386 #define DEFINE_STATIC_KEY_FALSE_RO(name) \ 387 struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ > 388 ANNOTATE_NOINSTR_ALLOWED(name) 389 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf 2024-11-25 7:08 ` kernel test robot 2024-11-25 8:00 ` kernel test robot @ 2024-11-25 10:47 ` kernel test robot 2 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2024-11-25 10:47 UTC (permalink / raw) To: Josh Poimboeuf, Valentin Schneider Cc: oe-kbuild-all, linux-kernel, Peter Zijlstra Hi Josh, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on v6.12 next-20241125] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Josh-Poimboeuf/jump_label-Add-annotations-for-validating-noinstr-usage/20241125-105905 base: linus/master patch link: https://lore.kernel.org/r/b5746646456eb030673cdb62c23d69ed83c2702a.1732326588.git.jpoimboe%40kernel.org patch subject: [PATCH 3/3] objtool: Add noinstr validation for static branches/calls config: i386-buildonly-randconfig-001-20241125 (https://download.01.org/0day-ci/archive/20241125/202411251822.MQI8kf80-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241125/202411251822.MQI8kf80-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411251822.MQI8kf80-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from include/linux/static_key.h:1, from arch/x86/include/asm/nospec-branch.h:6, from arch/x86/include/asm/paravirt_types.h:12, from arch/x86/include/asm/ptrace.h:175, from arch/x86/include/asm/math_emu.h:5, from arch/x86/include/asm/processor.h:13, from include/linux/sched.h:13, from include/linux/context_tracking.h:5, from arch/x86/kernel/kvm.c:12: include/linux/jump_label.h:388:9: warning: data definition has no type or storage class 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/kvm.c:48:1: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | DEFINE_STATIC_KEY_FALSE_RO(kvm_async_pf_enabled); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/jump_label.h:388:9: error: type defaults to 'int' in declaration of 'ANNOTATE_NOINSTR_ALLOWED' [-Werror=implicit-int] 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/kvm.c:48:1: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | DEFINE_STATIC_KEY_FALSE_RO(kvm_async_pf_enabled); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/jump_label.h:371:39: warning: parameter names (without types) in function declaration 371 | #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } | ^~~~~~~~~~~~~~~~ include/linux/jump_label.h:387:56: note: in expansion of macro 'STATIC_KEY_FALSE_INIT' 387 | struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ | ^~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/kvm.c:48:1: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | DEFINE_STATIC_KEY_FALSE_RO(kvm_async_pf_enabled); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors -- In file included from include/linux/static_key.h:1, from arch/x86/include/asm/nospec-branch.h:6, from arch/x86/include/asm/paravirt_types.h:12, from arch/x86/include/asm/ptrace.h:175, from arch/x86/include/asm/math_emu.h:5, from arch/x86/include/asm/processor.h:13, from include/linux/sched.h:13, from arch/x86/kernel/tsc.c:5: include/linux/jump_label.h:388:9: warning: data definition has no type or storage class 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/tsc.c:48:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/jump_label.h:388:9: error: type defaults to 'int' in declaration of 'ANNOTATE_NOINSTR_ALLOWED' [-Werror=implicit-int] 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/tsc.c:48:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/jump_label.h:371:39: warning: parameter names (without types) in function declaration 371 | #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } | ^~~~~~~~~~~~~~~~ include/linux/jump_label.h:387:56: note: in expansion of macro 'STATIC_KEY_FALSE_INIT' 387 | struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ | ^~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/tsc.c:48:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 48 | static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors -- In file included from include/linux/static_key.h:1, from arch/x86/include/asm/nospec-branch.h:6, from arch/x86/include/asm/irqflags.h:9, from include/linux/irqflags.h:18, from include/linux/spinlock.h:59, from include/linux/mmzone.h:8, from include/linux/gfp.h:7, from include/linux/mm.h:7, from include/linux/memblock.h:12, from arch/x86/kernel/cpu/common.c:5: include/linux/jump_label.h:388:9: warning: data definition has no type or storage class 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/cpu/common.c:404:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 404 | static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/jump_label.h:388:9: error: type defaults to 'int' in declaration of 'ANNOTATE_NOINSTR_ALLOWED' [-Werror=implicit-int] 388 | ANNOTATE_NOINSTR_ALLOWED(name) | ^~~~~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/cpu/common.c:404:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 404 | static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/jump_label.h:371:39: warning: parameter names (without types) in function declaration 371 | #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } | ^~~~~~~~~~~~~~~~ include/linux/jump_label.h:387:56: note: in expansion of macro 'STATIC_KEY_FALSE_INIT' 387 | struct static_key_false name __ro_after_init = STATIC_KEY_FALSE_INIT; \ | ^~~~~~~~~~~~~~~~~~~~~ arch/x86/kernel/cpu/common.c:404:8: note: in expansion of macro 'DEFINE_STATIC_KEY_FALSE_RO' 404 | static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +371 include/linux/jump_label.h 11276d5306b8e5 Peter Zijlstra 2015-07-24 369 11276d5306b8e5 Peter Zijlstra 2015-07-24 370 #define STATIC_KEY_TRUE_INIT (struct static_key_true) { .key = STATIC_KEY_INIT_TRUE, } 11276d5306b8e5 Peter Zijlstra 2015-07-24 @371 #define STATIC_KEY_FALSE_INIT (struct static_key_false){ .key = STATIC_KEY_INIT_FALSE, } 11276d5306b8e5 Peter Zijlstra 2015-07-24 372 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-11-25 10:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-23 1:52 [PATCH 0/3] objtool: noinstr validation for static branches/calls Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 1/3] jump_label: Add annotations for validating noinstr usage Josh Poimboeuf 2024-11-23 1:54 ` Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 2/3] static_call: Add read-only-after-init static calls Josh Poimboeuf 2024-11-23 1:52 ` [PATCH 3/3] objtool: Add noinstr validation for static branches/calls Josh Poimboeuf 2024-11-25 7:08 ` kernel test robot 2024-11-25 8:00 ` kernel test robot 2024-11-25 10:47 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox