Re: [PATCH v4 06/10] x86/traps: Decode LOCK Jcc.d8 #UD

[David Laight <david.laight.linux@gmail.com>]

On Mon, 24 Feb 2025 13:37:09 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> Because overlapping code sequences are all the rage.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Kees Cook <kees@kernel.org>
> ---
>  arch/x86/include/asm/bug.h |    2 ++
>  arch/x86/kernel/traps.c    |   26 +++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -17,6 +17,7 @@
>   * In clang we have UD1s reporting UBSAN failures on X86, 64 and 32bit.
>   */
>  #define INSN_ASOP		0x67
> +#define INSN_LOCK		0xf0
>  #define OPCODE_ESCAPE		0x0f
>  #define SECOND_BYTE_OPCODE_UD1	0xb9
>  #define SECOND_BYTE_OPCODE_UD2	0x0b
> @@ -26,6 +27,7 @@
>  #define BUG_UD1			0xfffd
>  #define BUG_UD1_UBSAN		0xfffc
>  #define BUG_EA			0xffea
> +#define BUG_LOCK		0xfff0
>  
>  #ifdef CONFIG_GENERIC_BUG
>  
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -97,6 +97,7 @@ __always_inline int is_valid_bugaddr(uns
>   * If it's a UD1, further decode to determine its use:
>   *
>   * FineIBT:      ea                      (bad)
> + * FineIBT:      0f 75 f9                lock jne . - 6
                    ^^ nibble swapped

	David

>   * UBSan{0}:     67 0f b9 00             ud1    (%eax),%eax
>   * UBSan{10}:    67 0f b9 40 10          ud1    0x10(%eax),%eax
>   * static_call:  0f b9 cc                ud1    %esp,%ecx
> @@ -106,6 +107,7 @@ __always_inline int is_valid_bugaddr(uns
>  __always_inline int decode_bug(unsigned long addr, s32 *imm, int *len)
>  {
>  	unsigned long start = addr;
> +	bool lock = false;
>  	u8 v;
>  
>  	if (addr < TASK_SIZE_MAX)
> @@ -114,12 +116,29 @@ __always_inline int decode_bug(unsigned
>  	v = *(u8 *)(addr++);
>  	if (v == INSN_ASOP)
>  		v = *(u8 *)(addr++);
> -	if (v == 0xea) {
> +
> +	if (v == INSN_LOCK) {
> +		lock = true;
> +		v = *(u8 *)(addr++);
> +	}
> +
> +	switch (v) {
> +	case 0x70 ... 0x7f: /* Jcc.d8 */
> +		addr += 1; /* d8 */
> +		*len = addr - start;
> +		WARN_ON_ONCE(!lock);
> +		return BUG_LOCK;
> +
> +	case 0xea:
>  		*len = addr - start;
>  		return BUG_EA;
> -	}
> -	if (v != OPCODE_ESCAPE)
> +
> +	case OPCODE_ESCAPE:
> +		break;
> +
> +	default:
>  		return BUG_NONE;
> +	}
>  
>  	v = *(u8 *)(addr++);
>  	if (v == SECOND_BYTE_OPCODE_UD2) {
> @@ -322,6 +341,7 @@ static noinstr bool handle_bug(struct pt
>  		fallthrough;
>  
>  	case BUG_EA:
> +	case BUG_LOCK:
>  		if (handle_cfi_failure(regs) == BUG_TRAP_TYPE_WARN) {
>  			handled = true;
>  			break;
> 
> 
>