| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aa3ff0f4-d74c-2c84-37c0-0880cabc0dd4@intel.com>
Date: Tue, 22 Mar 2022 00:05:31 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Dave Hansen <dave.hansen@...el.com>, <jpoimboe@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <x86@...nel.org>,
<linux-pm@...r.kernel.org>, <tglx@...utronix.de>,
<peterz@...radead.org>, <bp@...en8.de>, <rafael@...nel.org>,
<ravi.v.shankar@...el.com>
Subject: Re: [PATCH v2 1/2] x86/fpu: Add a helper to prepare AMX state for
low-power CPU idle
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;
}
Powered by blists - more mailing lists