Skip to content

Commit db2b0c5

Browse files
author
Peter Zijlstra
committed
objtool: Support pv_opsindirect calls for noinstr
Normally objtool will now follow indirect calls; there is no need. However, this becomes a problem with noinstr validation; if there's an indirect call from noinstr code, we very much need to know it is to another noinstr function. Luckily there aren't many indirect calls in entry code with the obvious exception of paravirt. As such, noinstr validation didn't work with paravirt kernels. In order to track pv_ops[] call targets, objtool reads the static pv_ops[] tables as well as direct assignments to the pv_ops[] array, provided the compiler makes them a single instruction like: bf87: 48 c7 05 00 00 00 00 00 00 00 00 movq $0x0,0x0(%rip) bf92 <xen_init_spinlocks+0x5f> bf8a: R_X86_64_PC32 pv_ops+0x268 There are, as of yet, no warnings for when this goes wrong :/ Using the functions found with the above means, all pv_ops[] calls are now subject to noinstr validation. Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 1462eb3 commit db2b0c5

File tree

7 files changed

+208
-13
lines changed

7 files changed

+208
-13
lines changed

lib/Kconfig.debug

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ config STACK_VALIDATION
458458

459459
config VMLINUX_VALIDATION
460460
bool
461-
depends on STACK_VALIDATION && DEBUG_ENTRY && !PARAVIRT
461+
depends on STACK_VALIDATION && DEBUG_ENTRY
462462
default y
463463

464464
config VMLINUX_MAP

tools/objtool/arch/x86/decode.c

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <objtool/arch.h>
2121
#include <objtool/warn.h>
2222
#include <objtool/endianness.h>
23+
#include <objtool/builtin.h>
2324
#include <arch/elf.h>
2425

2526
static int is_x86_64(const struct elf *elf)
@@ -102,12 +103,13 @@ unsigned long arch_jump_destination(struct instruction *insn)
102103
#define rm_is_mem(reg) (mod_is_mem() && !is_RIP() && rm_is(reg))
103104
#define rm_is_reg(reg) (mod_is_reg() && modrm_rm == (reg))
104105

105-
int arch_decode_instruction(const struct elf *elf, const struct section *sec,
106+
int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
106107
unsigned long offset, unsigned int maxlen,
107108
unsigned int *len, enum insn_type *type,
108109
unsigned long *immediate,
109110
struct list_head *ops_list)
110111
{
112+
const struct elf *elf = file->elf;
111113
struct insn insn;
112114
int x86_64, ret;
113115
unsigned char op1, op2,
@@ -544,6 +546,36 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
544546
*type = INSN_RETURN;
545547
break;
546548

549+
case 0xc7: /* mov imm, r/m */
550+
if (!noinstr)
551+
break;
552+
553+
if (insn.length == 3+4+4 && !strncmp(sec->name, ".init.text", 10)) {
554+
struct reloc *immr, *disp;
555+
struct symbol *func;
556+
int idx;
557+
558+
immr = find_reloc_by_dest(elf, (void *)sec, offset+3);
559+
disp = find_reloc_by_dest(elf, (void *)sec, offset+7);
560+
561+
if (!immr || strcmp(immr->sym->name, "pv_ops"))
562+
break;
563+
564+
idx = (immr->addend + 8) / sizeof(void *);
565+
566+
func = disp->sym;
567+
if (disp->sym->type == STT_SECTION)
568+
func = find_symbol_by_offset(disp->sym->sec, disp->addend);
569+
if (!func) {
570+
WARN("no func for pv_ops[]");
571+
return -1;
572+
}
573+
574+
objtool_pv_add(file, idx, func);
575+
}
576+
577+
break;
578+
547579
case 0xcf: /* iret */
548580
/*
549581
* Handle sync_core(), which has an IRET to self.

tools/objtool/check.c

Lines changed: 141 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ static int decode_instructions(struct objtool_file *file)
382382
insn->sec = sec;
383383
insn->offset = offset;
384384

385-
ret = arch_decode_instruction(file->elf, sec, offset,
385+
ret = arch_decode_instruction(file, sec, offset,
386386
sec->len - offset,
387387
&insn->len, &insn->type,
388388
&insn->immediate,
@@ -420,6 +420,82 @@ static int decode_instructions(struct objtool_file *file)
420420
return ret;
421421
}
422422

423+
/*
424+
* Read the pv_ops[] .data table to find the static initialized values.
425+
*/
426+
static int add_pv_ops(struct objtool_file *file, const char *symname)
427+
{
428+
struct symbol *sym, *func;
429+
unsigned long off, end;
430+
struct reloc *rel;
431+
int idx;
432+
433+
sym = find_symbol_by_name(file->elf, symname);
434+
if (!sym)
435+
return 0;
436+
437+
off = sym->offset;
438+
end = off + sym->len;
439+
for (;;) {
440+
rel = find_reloc_by_dest_range(file->elf, sym->sec, off, end - off);
441+
if (!rel)
442+
break;
443+
444+
func = rel->sym;
445+
if (func->type == STT_SECTION)
446+
func = find_symbol_by_offset(rel->sym->sec, rel->addend);
447+
448+
idx = (rel->offset - sym->offset) / sizeof(unsigned long);
449+
450+
objtool_pv_add(file, idx, func);
451+
452+
off = rel->offset + 1;
453+
if (off > end)
454+
break;
455+
}
456+
457+
return 0;
458+
}
459+
460+
/*
461+
* Allocate and initialize file->pv_ops[].
462+
*/
463+
static int init_pv_ops(struct objtool_file *file)
464+
{
465+
static const char *pv_ops_tables[] = {
466+
"pv_ops",
467+
"xen_cpu_ops",
468+
"xen_irq_ops",
469+
"xen_mmu_ops",
470+
NULL,
471+
};
472+
const char *pv_ops;
473+
struct symbol *sym;
474+
int idx, nr;
475+
476+
if (!noinstr)
477+
return 0;
478+
479+
file->pv_ops = NULL;
480+
481+
sym = find_symbol_by_name(file->elf, "pv_ops");
482+
if (!sym)
483+
return 0;
484+
485+
nr = sym->len / sizeof(unsigned long);
486+
file->pv_ops = calloc(sizeof(struct pv_state), nr);
487+
if (!file->pv_ops)
488+
return -1;
489+
490+
for (idx = 0; idx < nr; idx++)
491+
INIT_LIST_HEAD(&file->pv_ops[idx].targets);
492+
493+
for (idx = 0; (pv_ops = pv_ops_tables[idx]); idx++)
494+
add_pv_ops(file, pv_ops);
495+
496+
return 0;
497+
}
498+
423499
static struct instruction *find_last_insn(struct objtool_file *file,
424500
struct section *sec)
425501
{
@@ -893,6 +969,9 @@ static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *i
893969
return NULL;
894970

895971
if (!insn->reloc) {
972+
if (!file)
973+
return NULL;
974+
896975
insn->reloc = find_reloc_by_dest_range(file->elf, insn->sec,
897976
insn->offset, insn->len);
898977
if (!insn->reloc) {
@@ -1882,6 +1961,10 @@ static int decode_sections(struct objtool_file *file)
18821961

18831962
mark_rodata(file);
18841963

1964+
ret = init_pv_ops(file);
1965+
if (ret)
1966+
return ret;
1967+
18851968
ret = decode_instructions(file);
18861969
if (ret)
18871970
return ret;
@@ -2663,20 +2746,64 @@ static inline bool func_uaccess_safe(struct symbol *func)
26632746

26642747
static inline const char *call_dest_name(struct instruction *insn)
26652748
{
2749+
static char pvname[16];
2750+
struct reloc *rel;
2751+
int idx;
2752+
26662753
if (insn->call_dest)
26672754
return insn->call_dest->name;
26682755

2756+
rel = insn_reloc(NULL, insn);
2757+
if (rel && !strcmp(rel->sym->name, "pv_ops")) {
2758+
idx = (rel->addend / sizeof(void *));
2759+
snprintf(pvname, sizeof(pvname), "pv_ops[%d]", idx);
2760+
return pvname;
2761+
}
2762+
26692763
return "{dynamic}";
26702764
}
26712765

2672-
static inline bool noinstr_call_dest(struct symbol *func)
2766+
static bool pv_call_dest(struct objtool_file *file, struct instruction *insn)
2767+
{
2768+
struct symbol *target;
2769+
struct reloc *rel;
2770+
int idx;
2771+
2772+
rel = insn_reloc(file, insn);
2773+
if (!rel || strcmp(rel->sym->name, "pv_ops"))
2774+
return false;
2775+
2776+
idx = (arch_dest_reloc_offset(rel->addend) / sizeof(void *));
2777+
2778+
if (file->pv_ops[idx].clean)
2779+
return true;
2780+
2781+
file->pv_ops[idx].clean = true;
2782+
2783+
list_for_each_entry(target, &file->pv_ops[idx].targets, pv_target) {
2784+
if (!target->sec->noinstr) {
2785+
WARN("pv_ops[%d]: %s", idx, target->name);
2786+
file->pv_ops[idx].clean = false;
2787+
}
2788+
}
2789+
2790+
return file->pv_ops[idx].clean;
2791+
}
2792+
2793+
static inline bool noinstr_call_dest(struct objtool_file *file,
2794+
struct instruction *insn,
2795+
struct symbol *func)
26732796
{
26742797
/*
26752798
* We can't deal with indirect function calls at present;
26762799
* assume they're instrumented.
26772800
*/
2678-
if (!func)
2801+
if (!func) {
2802+
if (file->pv_ops)
2803+
return pv_call_dest(file, insn);
2804+
26792805
return false;
2806+
}
26802807

26812808
/*
26822809
* If the symbol is from a noinstr section; we good.
@@ -2695,10 +2822,12 @@ static inline bool noinstr_call_dest(struct symbol *func)
26952822
return false;
26962823
}
26972824

2698-
static int validate_call(struct instruction *insn, struct insn_state *state)
2825+
static int validate_call(struct objtool_file *file,
2826+
struct instruction *insn,
2827+
struct insn_state *state)
26992828
{
27002829
if (state->noinstr && state->instr <= 0 &&
2701-
!noinstr_call_dest(insn->call_dest)) {
2830+
!noinstr_call_dest(file, insn, insn->call_dest)) {
27022831
WARN_FUNC("call to %s() leaves .noinstr.text section",
27032832
insn->sec, insn->offset, call_dest_name(insn));
27042833
return 1;
@@ -2719,15 +2848,17 @@ static int validate_call(struct instruction *insn, struct insn_state *state)
27192848
return 0;
27202849
}
27212850

2722-
static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
2851+
static int validate_sibling_call(struct objtool_file *file,
2852+
struct instruction *insn,
2853+
struct insn_state *state)
27232854
{
27242855
if (has_modified_stack_frame(insn, state)) {
27252856
WARN_FUNC("sibling call from callable instruction with modified stack frame",
27262857
insn->sec, insn->offset);
27272858
return 1;
27282859
}
27292860

2730-
return validate_call(insn, state);
2861+
return validate_call(file, insn, state);
27312862
}
27322863

27332864
static int validate_return(struct symbol *func, struct instruction *insn, struct insn_state *state)
@@ -2880,7 +3011,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
28803011

28813012
case INSN_CALL:
28823013
case INSN_CALL_DYNAMIC:
2883-
ret = validate_call(insn, &state);
3014+
ret = validate_call(file, insn, &state);
28843015
if (ret)
28853016
return ret;
28863017

@@ -2899,7 +3030,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
28993030
case INSN_JUMP_CONDITIONAL:
29003031
case INSN_JUMP_UNCONDITIONAL:
29013032
if (is_sibling_call(insn)) {
2902-
ret = validate_sibling_call(insn, &state);
3033+
ret = validate_sibling_call(file, insn, &state);
29033034
if (ret)
29043035
return ret;
29053036

@@ -2921,7 +3052,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
29213052
case INSN_JUMP_DYNAMIC:
29223053
case INSN_JUMP_DYNAMIC_CONDITIONAL:
29233054
if (is_sibling_call(insn)) {
2924-
ret = validate_sibling_call(insn, &state);
3055+
ret = validate_sibling_call(file, insn, &state);
29253056
if (ret)
29263057
return ret;
29273058
}

tools/objtool/include/objtool/arch.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ struct instruction;
6969

7070
void arch_initial_func_cfi_state(struct cfi_init_state *state);
7171

72-
int arch_decode_instruction(const struct elf *elf, const struct section *sec,
72+
int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
7373
unsigned long offset, unsigned int maxlen,
7474
unsigned int *len, enum insn_type *type,
7575
unsigned long *immediate,

tools/objtool/include/objtool/elf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ struct symbol {
5757
struct symbol *pfunc, *cfunc, *alias;
5858
bool uaccess_safe;
5959
bool static_call_tramp;
60+
struct list_head pv_target;
6061
};
6162

6263
struct reloc {

tools/objtool/include/objtool/objtool.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@
1414

1515
#define __weak __attribute__((weak))
1616

17+
struct pv_state {
18+
bool clean;
19+
struct list_head targets;
20+
};
21+
1722
struct objtool_file {
1823
struct elf *elf;
1924
struct list_head insn_list;
@@ -25,10 +30,14 @@ struct objtool_file {
2530

2631
unsigned long jl_short, jl_long;
2732
unsigned long jl_nop_short, jl_nop_long;
33+
34+
struct pv_state *pv_ops;
2835
};
2936

3037
struct objtool_file *objtool_open_read(const char *_objname);
3138

39+
void objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
40+
3241
int check(struct objtool_file *file);
3342
int orc_dump(const char *objname);
3443
int orc_create(struct objtool_file *file);

tools/objtool/objtool.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,28 @@ struct objtool_file *objtool_open_read(const char *_objname)
135135
return &file;
136136
}
137137

138+
void objtool_pv_add(struct objtool_file *f, int idx, struct symbol *func)
139+
{
140+
if (!noinstr)
141+
return;
142+
143+
if (!f->pv_ops) {
144+
WARN("paravirt confusion");
145+
return;
146+
}
147+
148+
/*
149+
* These functions will be patched into native code,
150+
* see paravirt_patch().
151+
*/
152+
if (!strcmp(func->name, "_paravirt_nop") ||
153+
!strcmp(func->name, "_paravirt_ident_64"))
154+
return;
155+
156+
list_add(&func->pv_target, &f->pv_ops[idx].targets);
157+
f->pv_ops[idx].clean = false;
158+
}
159+
138160
static void cmd_usage(void)
139161
{
140162
unsigned int i, longest = 0;

0 commit comments

Comments
 (0)