| Message ID | 20230720163056.2564824-13-vschneid@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | context_tracking,x86: Defer some IPIs until a user->kernel transition | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Jul 20, 2023 at 05:30:48PM +0100, Valentin Schneider wrote: > +static int validate_static_key(struct instruction *insn, struct insn_state *state) > +{ > + if (state->noinstr && state->instr <= 0) { > + if ((strcmp(insn->key_sym->sec->name, ".data..ro_after_init"))) { > + WARN_INSN(insn, > + "Non __ro_after_init static key \"%s\" in .noinstr section", For consistency with other warnings, this should start with a lowercase "n" and the string literal should be on the same line as the WARN_INSN, like WARN_INSN(insn, "non __ro_after_init static key \"%s\" in .noinstr section", ... > diff --git a/tools/objtool/special.c b/tools/objtool/special.c > index 91b1950f5bd8a..1f76cfd815bf3 100644 > --- a/tools/objtool/special.c > +++ b/tools/objtool/special.c > @@ -127,6 +127,9 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, > return -1; > } > alt->key_addend = reloc_addend(key_reloc); > + > + reloc_to_sec_off(key_reloc, &sec, &offset); > + alt->key_sym = find_symbol_by_offset(sec, offset & ~2); Bits 0 and 1 can both store data, should be ~3?
On Thu, Jul 20, 2023 at 05:30:48PM +0100, Valentin Schneider wrote: > Later commits will depend on having no runtime-mutable text in early entry > code. (ab)use the .noinstr section as a marker of early entry code and warn > about static keys used in it that can be flipped at runtime. Similar to my comment on patch 13, this could also use a short justification for adding the feature, i.e. why runtime-mutable text isn't going to be allowed in .noinstr. Also, please add a short description of the warning (and why it exists) to tools/objtool/Documentation/objtool.txt.
On 28/07/23 11:02, Josh Poimboeuf wrote: > On Thu, Jul 20, 2023 at 05:30:48PM +0100, Valentin Schneider wrote: >> Later commits will depend on having no runtime-mutable text in early entry >> code. (ab)use the .noinstr section as a marker of early entry code and warn >> about static keys used in it that can be flipped at runtime. > > Similar to my comment on patch 13, this could also use a short > justification for adding the feature, i.e. why runtime-mutable text > isn't going to be allowed in .noinstr. > > Also, please add a short description of the warning (and why it exists) > to tools/objtool/Documentation/objtool.txt. > I had missed this, thanks for the pointer. > -- > Josh
On 28/07/23 10:35, Josh Poimboeuf wrote: > On Thu, Jul 20, 2023 at 05:30:48PM +0100, Valentin Schneider wrote: >> +static int validate_static_key(struct instruction *insn, struct insn_state *state) >> +{ >> + if (state->noinstr && state->instr <= 0) { >> + if ((strcmp(insn->key_sym->sec->name, ".data..ro_after_init"))) { >> + WARN_INSN(insn, >> + "Non __ro_after_init static key \"%s\" in .noinstr section", > > For consistency with other warnings, this should start with a lowercase > "n" and the string literal should be on the same line as the WARN_INSN, > like > > WARN_INSN(insn, "non __ro_after_init static key \"%s\" in .noinstr section", > ... > >> diff --git a/tools/objtool/special.c b/tools/objtool/special.c >> index 91b1950f5bd8a..1f76cfd815bf3 100644 >> --- a/tools/objtool/special.c >> +++ b/tools/objtool/special.c >> @@ -127,6 +127,9 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, >> return -1; >> } >> alt->key_addend = reloc_addend(key_reloc); >> + >> + reloc_to_sec_off(key_reloc, &sec, &offset); >> + alt->key_sym = find_symbol_by_offset(sec, offset & ~2); > > Bits 0 and 1 can both store data, should be ~3? > Quite so, that needs to be the same as jump_entry_key(). > -- > Josh
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index d308330f2910e..d973bb4df4341 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1968,6 +1968,9 @@ static int add_special_section_alts(struct objtool_file *file) alt->next = orig_insn->alts; orig_insn->alts = alt; + if (special_alt->key_sym) + orig_insn->key_sym = special_alt->key_sym; + list_del(&special_alt->list); free(special_alt); } @@ -3476,6 +3479,20 @@ 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) { + if ((strcmp(insn->key_sym->sec->name, ".data..ro_after_init"))) { + WARN_INSN(insn, + "Non __ro_after_init static key \"%s\" in .noinstr section", + insn->key_sym->name); + return 1; + } + } + + return 0; +} + static struct instruction *next_insn_to_validate(struct objtool_file *file, struct instruction *insn) { @@ -3625,6 +3642,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_sym) + 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 daa46f1f0965a..35dd21f8f41e1 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_sym; }; static inline struct symbol *insn_func(struct instruction *insn) diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h index 86d4af9c5aa9d..0e61f34fe3a28 100644 --- a/tools/objtool/include/objtool/special.h +++ b/tools/objtool/include/objtool/special.h @@ -27,6 +27,8 @@ struct special_alt { struct section *new_sec; unsigned long new_off; + struct symbol *key_sym; + unsigned int orig_len, new_len; /* group only */ }; diff --git a/tools/objtool/special.c b/tools/objtool/special.c index 91b1950f5bd8a..1f76cfd815bf3 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -127,6 +127,9 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, return -1; } alt->key_addend = reloc_addend(key_reloc); + + reloc_to_sec_off(key_reloc, &sec, &offset); + alt->key_sym = find_symbol_by_offset(sec, offset & ~2); } return 0;
Later commits will depend on having no runtime-mutable text in early entry code. (ab)use the .noinstr section as a marker of early entry code and warn about static keys used in it that can be flipped at runtime. Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Valentin Schneider <vschneid@redhat.com> --- tools/objtool/check.c | 20 ++++++++++++++++++++ tools/objtool/include/objtool/check.h | 1 + tools/objtool/include/objtool/special.h | 2 ++ tools/objtool/special.c | 3 +++ 4 files changed, 26 insertions(+)