linux-kernel - Re: [PATCH 02/10] x86/insn: Fix PUSH instruction in x86 instruction decoder opcode map

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 2 May 2024 10:41:14 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: linux-kernel@...r.kernel.org, "Chang S. Bae" <chang.seok.bae@...el.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Nikolay Borisov <nik.borisov@...e.com>,
	Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
	Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
	Ian Rogers <irogers@...gle.com>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 02/10] x86/insn: Fix PUSH instruction in x86 instruction
 decoder opcode map

On Thu, May 02, 2024 at 01:58:45PM +0300, Adrian Hunter wrote:
> The x86 instruction decoder is used not only for decoding kernel
> instructions. It is also used by perf uprobes (user space probes) and by
> perf tools Intel Processor Trace decoding. Consequently, it needs to
> support instructions executed by user space also.

I wonder if we should do it this way, i.e. updating both tools/ and
kernel source code in the same patch.

I think the best is to update the kernel bits, then, after that is
merged, do the tooling part.

To avoid possible, yet unlikely, clashes in linux-next, for instance.

- Arnaldo
 
> Opcode 0x68 PUSH instruction is currently defined as 64-bit operand size
> only i.e. (d64). That was based on Intel SDM Opcode Map. However that is
> contradicted by the Instruction Set Reference section for PUSH in the
> same manual.
> 
> Remove 64-bit operand size only annotation from opcode 0x68 PUSH
> instruction.
> 
> Example:
> 
>   $ cat pushw.s
>   .global  _start
>   .text
>   _start:
>           pushw   $0x1234
>           mov     $0x1,%eax   # system call number (sys_exit)
>           int     $0x80
>   $ as -o pushw.o pushw.s
>   $ ld -s -o pushw pushw.o
>   $ objdump -d pushw | tail -4
>   0000000000401000 <.text>:
>     401000:       66 68 34 12             pushw  $0x1234
>     401004:       b8 01 00 00 00          mov    $0x1,%eax
>     401009:       cd 80                   int    $0x80
>   $ perf record -e intel_pt//u ./pushw
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.014 MB perf.data ]
> 
>  Before:
> 
>   $ perf script --insn-trace=disasm
>   Warning:
>   1 instruction trace errors
>            pushw   10349 [000] 10586.869237014:            401000 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           pushw $0x1234
>            pushw   10349 [000] 10586.869237014:            401006 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb %al, (%rax)
>            pushw   10349 [000] 10586.869237014:            401008 [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb %cl, %ch
>            pushw   10349 [000] 10586.869237014:            40100a [unknown] (/home/ahunter/git/misc/rtit-tests/pushw)           addb $0x2e, (%rax)
>    instruction trace error type 1 time 10586.869237224 cpu 0 pid 10349 tid 10349 ip 0x40100d code 6: Trace doesn't match instruction
> 
>  After:
> 
>   $ perf script --insn-trace=disasm
>              pushw   10349 [000] 10586.869237014:            401000 [unknown] (./pushw)           pushw $0x1234
>              pushw   10349 [000] 10586.869237014:            401004 [unknown] (./pushw)           movl $1, %eax
> 
> Fixes: eb13296cfaf6 ("x86: Instruction decoder API")
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
>  arch/x86/lib/x86-opcode-map.txt       | 2 +-
>  tools/arch/x86/lib/x86-opcode-map.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
> index c94988d5130d..4ea2e6adb477 100644
> --- a/arch/x86/lib/x86-opcode-map.txt
> +++ b/arch/x86/lib/x86-opcode-map.txt
> @@ -148,7 +148,7 @@ AVXcode:
>  65: SEG=GS (Prefix)
>  66: Operand-Size (Prefix)
>  67: Address-Size (Prefix)
> -68: PUSH Iz (d64)
> +68: PUSH Iz
>  69: IMUL Gv,Ev,Iz
>  6a: PUSH Ib (d64)
>  6b: IMUL Gv,Ev,Ib
> diff --git a/tools/arch/x86/lib/x86-opcode-map.txt b/tools/arch/x86/lib/x86-opcode-map.txt
> index c94988d5130d..4ea2e6adb477 100644
> --- a/tools/arch/x86/lib/x86-opcode-map.txt
> +++ b/tools/arch/x86/lib/x86-opcode-map.txt
> @@ -148,7 +148,7 @@ AVXcode:
>  65: SEG=GS (Prefix)
>  66: Operand-Size (Prefix)
>  67: Address-Size (Prefix)
> -68: PUSH Iz (d64)
> +68: PUSH Iz
>  69: IMUL Gv,Ev,Iz
>  6a: PUSH Ib (d64)
>  6b: IMUL Gv,Ev,Ib
> -- 
> 2.34.1