Re: [RFC PATCH v2 20/21] x86: Add support for CONFIG_CFI_CLANG

On Mon, May 16, 2022 at 1:32 AM David Laight [off-list ref] wrote:

From: Sami Tolvanen

quoted

Sent: 13 May 2022 21:22

With CONFIG_CFI_CLANG, the compiler injects a type preamble
immediately before each function and a check to validate the target
function type before indirect calls:

  ; type preamble
  __cfi_function:
    int3
    int3
    mov <id>, %eax

Interesting - since this code can't be executed there is no
point adding an instruction 'prefix' to the 32bit constant.

The reason to embed the type into an instruction is to avoid the need
to special case objtool's instruction decoder.

quoted

    int3
    int3
  function:
    ...
  ; indirect call check
    cmpl    <id>, -6(%r11)
    je      .Ltmp1
    ud2
  .Ltmp1:
    call    __x86_indirect_thunk_r11

Define the __CFI_TYPE helper macro for manual type annotations in
assembly code, add error handling for the CFI ud2 traps, and allow
CONFIG_CFI_CLANG to be selected on x86_64.

...

quoted

+
+     /*
+      * The compiler generates the following instruction sequence
+      * for indirect call checks:
+      *
+      *   cmpl    <id>, -6(%reg)     ; 7 bytes

If the <id> is between -128 and 127 then an 8bit constant
(sign extended) might be used.
Possibly the compiler forces the assembler to generate the
long form.

There could also be a REX prefix.
That will break any code that tries to use %reg.

The compiler always generates this specific instruction sequence.

quoted

+      *   je      .Ltmp1             ; 2 bytes
+      *   ud2                        ; <- addr
+      *   .Ltmp1:
+      *
+      * Both the type and the target address can be decoded from the
+      * cmpl instruction.
+      */
+     if (copy_from_kernel_nofault(buffer, (void *)regs->ip - 9, MAX_INSN_SIZE))
+             return;
+     if (insn_decode_kernel(&insn, buffer))
+             return;
+     if (insn.opcode.value != 0x81 || X86_MODRM_REG(insn.modrm.value) != 7)
+             return;

Since you are looking for a very specific opcode why bother
calling insn_decode_kernel() - just check for the required (masked)
byte values.

Because I need to decode both the immediate value and the register
from that instruction.

quoted

+
+     *type = insn.immediate.value;
+
+     offset = insn_get_modrm_rm_off(&insn, regs);

Given the expected instruction, isn't that -6 ??

No, this is the register offset.

quoted

+     if (offset < 0)
+             return;
+
+     *target = *(unsigned long *)((void *)regs + offset);

WTF is that calculating??

It's reading the register value from pt_regs.

Sami

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel