[RESEND PATCH v2 0/2] *** Fix reformat_objdump.awk ***

[RESEND PATCH v2 0/2] *** Fix reformat_objdump.awk ***

[Samuel Zeter]

These are two small patches which originally dealt with
the problem found at:

https://github.com/ClangBuiltLinux/linux/issues/1364

The original steps to reproduce were:
$ make -skj"$(nproc)" LLVM=1 LLVM_IAS=1 defconfig
$ scripts/config -e X86_DECODER_SELFTEST
$ make -skj"$(nproc)" LLVM=1 LLVM_IAS=1 olddefconfig bzImage

Which resulted in the error:
arch/x86/tools/insn_decoder_test: warning: objdump says 0 bytes, but
insn_get_length() says 2

Upon inspection it turned out llvm-objdump was formatting its
output differently, which caused objdump_reformat.awk to incorrectly
output its values.

After fixing that bug, a second one was seen where the instruction
"wait" was incorrectly matched with "fwait", which again caused
insn_decoder_test to fail.

Samuel Zeter (2):
  arch/x86/tools/objdump_reformat.awk: Ensure regex matches fwait
  arch/x86/tools/objdump_reformat.awk: Allow for spaces

 arch/x86/tools/objdump_reformat.awk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 151297128990b4b44be8d70b983053831fa857f6
-- 
2.38.1

[RESEND PATCH v2 1/2] arch/x86/tools/objdump_reformat.awk: Ensure regex matches fwait

[Samuel Zeter]

If there is "wait" mnemonic in the line being parsed, it is incorrectly
handled by the script, and an extra line of "fwait" in
objdump_reformat's output is inserted. As insn_decoder_test relies
upon the formatted output, the test fails.

This is reproducible when disassembling with llvm-objdump:

Pre-processed lines:
ffffffff81033e72: 9b                    wait
ffffffff81033e73: 48 c7 c7 89 50 42 82  movq

After objdump_reformat.awk:
ffffffff81033e72:       9b      fwait
ffffffff81033e72:                               wait
ffffffff81033e73:       48 c7 c7 89 50 42 82    movq

The regex match now accepts spaces or tabs, along with the "fwait"
instruction.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Samuel Zeter <samuelzeter@gmail.com>
---
 arch/x86/tools/objdump_reformat.awk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/tools/objdump_reformat.awk b/arch/x86/tools/objdump_reformat.awk
index f418c91b71f0..276e572a6f60 100644
--- a/arch/x86/tools/objdump_reformat.awk
+++ b/arch/x86/tools/objdump_reformat.awk
@@ -12,7 +12,7 @@ BEGIN {
 	prev_hex = ""
 	prev_mnemonic = ""
 	bad_expr = "(\\(bad\\)|^rex|^.byte|^rep(z|nz)$|^lock$|^es$|^cs$|^ss$|^ds$|^fs$|^gs$|^data(16|32)$|^addr(16|32|64))"
-	fwait_expr = "^9b "
+	fwait_expr = "^9b[ \t]*fwait"
 	fwait_str="9b\tfwait"
 }
 
-- 
2.38.1

[RESEND PATCH v2 2/2] arch/x86/tools/objdump_reformat.awk: Allow for spaces

[Samuel Zeter]

objdump and llvm-objdump have differing output formats. Specifically,
objump will format its output as: address:<tab>hex, whereas
llvm-objdump displays its output as address:<space>hex.

objdump_reformat.awk incorrectly handles this discrepancy due to
the unexpected space and as a result insn_decoder_test fails, as
its input is garbled.

The instruction line being tokenized now handles a space and colon,
or tab delimiter.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Samuel Zeter <samuelzeter@gmail.com>
---
 arch/x86/tools/objdump_reformat.awk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/tools/objdump_reformat.awk b/arch/x86/tools/objdump_reformat.awk
index 276e572a6f60..a4120d907277 100644
--- a/arch/x86/tools/objdump_reformat.awk
+++ b/arch/x86/tools/objdump_reformat.awk
@@ -22,7 +22,7 @@ BEGIN {
 }
 
 /^ *[0-9a-f]+:/ {
-	if (split($0, field, "\t") < 3) {
+	if (split($0, field, /: |\t/) < 3) {
 		# This is a continuation of the same insn.
 		prev_hex = prev_hex field[2]
 	} else {
-- 
2.38.1

Re: [RESEND PATCH v2 0/2] *** Fix reformat_objdump.awk ***

[Kees Cook]

*thread necromancy*

These patches still apply and are still needed to correctly process
objdump output. It accidentally doesn't produce any errors currently,
but manual inspection shows mangled results.

Can an x86 maintainer take these two patches please? I can carry them if
that's preferred?

Note that there is an older Ack from Masami:

https://lore.kernel.org/all/20220107205259.e66ff1bd90ac218611bf1178@kernel.org/

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

And I've now reviewed and tested this too:

Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

On Sat, Nov 12, 2022 at 07:04:15PM +1000, Samuel Zeter wrote:
> These are two small patches which originally dealt with
> the problem found at:
> 
> https://github.com/ClangBuiltLinux/linux/issues/1364
> 
> The original steps to reproduce were:
> $ make -skj"$(nproc)" LLVM=1 LLVM_IAS=1 defconfig
> $ scripts/config -e X86_DECODER_SELFTEST
> $ make -skj"$(nproc)" LLVM=1 LLVM_IAS=1 olddefconfig bzImage
> 
> Which resulted in the error:
> arch/x86/tools/insn_decoder_test: warning: objdump says 0 bytes, but
> insn_get_length() says 2
> 
> Upon inspection it turned out llvm-objdump was formatting its
> output differently, which caused objdump_reformat.awk to incorrectly
> output its values.
> 
> After fixing that bug, a second one was seen where the instruction
> "wait" was incorrectly matched with "fwait", which again caused
> insn_decoder_test to fail.
> 
> Samuel Zeter (2):
>   arch/x86/tools/objdump_reformat.awk: Ensure regex matches fwait
>   arch/x86/tools/objdump_reformat.awk: Allow for spaces
> 
>  arch/x86/tools/objdump_reformat.awk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> base-commit: 151297128990b4b44be8d70b983053831fa857f6
> -- 
> 2.38.1
> 

-- 
Kees Cook