Messages in this thread Patches in this message | | | Date | Tue, 22 Mar 2022 00:05:31 -0700 | Subject | Re: [PATCH v2 1/2] x86/fpu: Add a helper to prepare AMX state for low-power CPU idle | From | "Chang S. Bae" <> |
| |
On 3/10/2022 1:00 PM, Chang S. Bae wrote: > > BTW, now I'm suspicious of this JMP as patched at runtime with > fpu_state_size_dynamic():
No, this jump was supposed to be replaced with NOP by objtool but it didn't as fail to interpret TILERELEASE in this case.
> > 22: eb 01 jmp 0x25 > 24: c3 retq > 25: b9 01 00 00 00 mov $0x1,%ecx > 2a:* 0f 01 d0 xgetbv <-- trapping instruction > > Still, the question is, if so, why it was patched on non-XFD systems. > Let me analyze the case a bit further with 0day folks. >
Looks like 0day picked an internal branch where the instruction's opcode was intentionally removed.
In practice, upstream code should accompany by a complete opcode table.
If it ever happens, a warning follows like this on build: arch/x86/kernel/fpu/core.o: warning: objtool: can't decode instruction at .text:0x185e
But what actually happened is barely indicated by this message alone. This decode failure ends up returning check() immediately [1] so the file is entirely skipped from the tool's process.
I came to think of some improvements for this tool:
(1) Add more messages like [2]. This may help users understand what happens in this build process.
(2) Move on next byte from the failed offset like [3]. Perhaps, this continuation may alleviate the impact. It may misinterpret some bytes but I think it will be re-aligned with padding bytes before the next function (symbol).
Include Josh Poimboeuf. Appreciate any feedback.
Thanks, Chang
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/objtool/check.c#n3515
[2]:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7c33ec67c4a9..34b60fa33fbe 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -3507,27 +3507,27 @@ int check(struct objtool_file *file) set_func_state(&func_cfi);
if (!cfi_hash_alloc(1UL << (file->elf->symbol_bits - 3))) - goto out; + goto err;
cfi_hash_add(&init_cfi); cfi_hash_add(&func_cfi);
ret = decode_sections(file); if (ret < 0) - goto out; + goto err;
warnings += ret;
if (list_empty(&file->insn_list)) - goto out; + return 0;
if (vmlinux && !validate_dup) { ret = validate_vmlinux_functions(file); if (ret < 0) - goto out; + goto err;
warnings += ret; - goto out; + return 0; }
if (retpoline) { @@ -3539,37 +3539,37 @@ int check(struct objtool_file *file)
ret = validate_functions(file); if (ret < 0) - goto out; + goto err; warnings += ret;
ret = validate_unwind_hints(file, NULL); if (ret < 0) - goto out; + goto err; warnings += ret;
if (!warnings) { ret = validate_reachable_instructions(file); if (ret < 0) - goto out; + goto err; warnings += ret; }
ret = create_static_call_sections(file); if (ret < 0) - goto out; + goto err; warnings += ret;
if (retpoline) { ret = create_retpoline_sites_sections(file); if (ret < 0) - goto out; + goto err; warnings += ret; }
if (mcount) { ret = create_mcount_loc_sections(file); if (ret < 0) - goto out; + goto err; warnings += ret; }
@@ -3580,11 +3580,14 @@ int check(struct objtool_file *file) printf("nr_cfi_cache: %ld\n", nr_cfi_cache); }
-out: + return 0; + +err: /* * For now, don't fail the kernel build on fatal warnings.These * errors are still fairly common due to the growing matrix of * supported toolchains and their recent pace of change. */ + WARN("check failed - no jump_label instructions were written."); return 0; } [3]:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7c33ec67c4a9..1f1515373ca5 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -371,7 +371,7 @@ static int decode_instructions(struct objtool_file *file) !strcmp(sec->name, ".entry.text")) sec->noinstr = true;
- for (offset = 0; offset < sec->sh.sh_size; offset += insn->len) { + for (offset = 0; offset < sec->sh.sh_size;) { insn = malloc(sizeof(*insn)); if (!insn) { WARN("malloc failed"); @@ -389,12 +389,15 @@ static int decode_instructions(struct objtool_file *file) &insn->len, &insn->type, &insn->immediate, &insn->stack_ops); - if (ret) - goto err; + if (ret) { + offset++; + continue; + }
hash_add(file->insn_hash, &insn->hash, sec_offset_hash(sec, insn->offset)); list_add_tail(&insn->list, &file->insn_list); nr_insns++; + offset += insn->len; }
list_for_each_entry(func, &sec->symbol_list, list) { @@ -416,10 +419,6 @@ static int decode_instructions(struct objtool_file *file) printf("nr_insns: %lu\n", nr_insns);
return 0; - -err: - free(insn); - return ret; }
| |