Decode ENDBR instructions and WARN about NOTRACK prefixes.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
tools/objtool/arch/x86/decode.c | 34 +++++++++++++++++++++++++++++-----
tools/objtool/include/objtool/arch.h | 1 +
2 files changed, 30 insertions(+), 5 deletions(-)
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -103,6 +103,18 @@ unsigned long arch_jump_destination(stru
#define rm_is_mem(reg) (mod_is_mem() && !is_RIP() && rm_is(reg))
#define rm_is_reg(reg) (mod_is_reg() && modrm_rm == (reg))
+static bool has_notrack_prefix(struct insn *insn)
+{
+ int i;
+
+ for (i = 0; i < insn->prefixes.nbytes; i++) {
+ if (insn->prefixes.bytes[i] == 0x3e)
+ return true;
+ }
+
+ return false;
+}
+
int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
@@ -112,7 +124,7 @@ int arch_decode_instruction(struct objto
const struct elf *elf = file->elf;
struct insn insn;
int x86_64, ret;
- unsigned char op1, op2, op3,
+ unsigned char op1, op2, op3, prefix,
rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, rex_x = 0,
modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0,
sib = 0, /* sib_scale = 0, */ sib_index = 0, sib_base = 0;
@@ -137,6 +149,8 @@ int arch_decode_instruction(struct objto
if (insn.vex_prefix.nbytes)
return 0;
+ prefix = insn.prefixes.bytes[0];
+
op1 = insn.opcode.bytes[0];
op2 = insn.opcode.bytes[1];
op3 = insn.opcode.bytes[2];
@@ -492,6 +506,12 @@ int arch_decode_instruction(struct objto
/* nopl/nopw */
*type = INSN_NOP;
+ } else if (op2 == 0x1e) {
+
+ if (prefix == 0xf3 && (modrm == 0xfa || modrm == 0xfb))
+ *type = INSN_ENDBR;
+
+
} else if (op2 == 0x38 && op3 == 0xf8) {
if (insn.prefixes.nbytes == 1 &&
insn.prefixes.bytes[0] == 0xf2) {
@@ -636,20 +656,24 @@ int arch_decode_instruction(struct objto
break;
case 0xff:
- if (modrm_reg == 2 || modrm_reg == 3)
+ if (modrm_reg == 2 || modrm_reg == 3) {
*type = INSN_CALL_DYNAMIC;
+ if (has_notrack_prefix(&insn))
+ WARN("notrack prefix found at %s:0x%lx", sec->name, offset);
- else if (modrm_reg == 4)
+ } else if (modrm_reg == 4) {
*type = INSN_JUMP_DYNAMIC;
+ if (has_notrack_prefix(&insn))
+ WARN("notrack prefix found at %s:0x%lx", sec->name, offset);
- else if (modrm_reg == 5)
+ } else if (modrm_reg == 5) {
/* jmpf */
*type = INSN_CONTEXT_SWITCH;
- else if (modrm_reg == 6) {
+ } else if (modrm_reg == 6) {
/* push from mem */
ADD_OP(op) {
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -27,6 +27,7 @@ enum insn_type {
INSN_STD,
INSN_CLD,
INSN_TRAP,
+ INSN_ENDBR,
INSN_OTHER,
};
On Thu, 3 Mar 2022, Andrew Cooper wrote:
> On 03/03/2022 10:53, Miroslav Benes wrote:
> > Hi,
> >
> > On Thu, 24 Feb 2022, Peter Zijlstra wrote:
> >
> >> Decode ENDBR instructions and WARN about NOTRACK prefixes.
> > I guess it has been already mentioned somewhere, but could you explain
> > NOTRACK prefix here, please? If I understand it right, it disables IBT for
> > the indirect branch instruction meaning that its target does not have to
> > start with ENDBR?
>
> CET-IBT has loads of get-out clauses. The NOTRACK prefix is one; the
> legacy code bitmap (implicit NOTRACK for whole libraries) is another.
>
> And yes - the purpose of NOTRACK is to exempt a specific indirect branch
> from checks.
>
> GCC can emit NOTRACK'd calls in some cases when e.g. the programmer
> launders a function pointer through (void *), or when
> __attribute__((no_cf_check)) is used explicitly.
>
>
> Each of the get-out clauses has separate enable bits, as each of them
> reduces security. In this series, Linux sets MSR_S_CET.ENDBR_EN but
> specifically does not set NOTRACK_EN, so NOTRACK prefixes will be
> ignored and suffer #CP if encountered.
Thanks for the explanation. I would be nice to include it somewhere so
that it is not lost.
Miroslav
Hi,
On Thu, 24 Feb 2022, Peter Zijlstra wrote:
> Decode ENDBR instructions and WARN about NOTRACK prefixes.
I guess it has been already mentioned somewhere, but could you explain
NOTRACK prefix here, please? If I understand it right, it disables IBT for
the indirect branch instruction meaning that its target does not have to
start with ENDBR?
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> tools/objtool/arch/x86/decode.c | 34 +++++++++++++++++++++++++++++-----
> tools/objtool/include/objtool/arch.h | 1 +
> 2 files changed, 30 insertions(+), 5 deletions(-)
>
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -103,6 +103,18 @@ unsigned long arch_jump_destination(stru
> #define rm_is_mem(reg) (mod_is_mem() && !is_RIP() && rm_is(reg))
> #define rm_is_reg(reg) (mod_is_reg() && modrm_rm == (reg))
>
> +static bool has_notrack_prefix(struct insn *insn)
> +{
> + int i;
> +
> + for (i = 0; i < insn->prefixes.nbytes; i++) {
> + if (insn->prefixes.bytes[i] == 0x3e)
> + return true;
> + }
> +
> + return false;
> +}
> +
...
> @@ -636,20 +656,24 @@ int arch_decode_instruction(struct objto
> break;
>
> case 0xff:
> - if (modrm_reg == 2 || modrm_reg == 3)
> + if (modrm_reg == 2 || modrm_reg == 3) {
>
> *type = INSN_CALL_DYNAMIC;
> + if (has_notrack_prefix(&insn))
> + WARN("notrack prefix found at %s:0x%lx", sec->name, offset);
>
> - else if (modrm_reg == 4)
> + } else if (modrm_reg == 4) {
>
> *type = INSN_JUMP_DYNAMIC;
> + if (has_notrack_prefix(&insn))
> + WARN("notrack prefix found at %s:0x%lx", sec->name, offset);
And we want to warn about it here so that we can have it all in the kernel
control?
Miroslav
On Thu, Mar 03, 2022 at 01:33:06PM +0100, Miroslav Benes wrote:
> On Thu, 3 Mar 2022, Andrew Cooper wrote:
>
> > On 03/03/2022 10:53, Miroslav Benes wrote:
> > > Hi,
> > >
> > > On Thu, 24 Feb 2022, Peter Zijlstra wrote:
> > >
> > >> Decode ENDBR instructions and WARN about NOTRACK prefixes.
> > > I guess it has been already mentioned somewhere, but could you explain
> > > NOTRACK prefix here, please? If I understand it right, it disables IBT for
> > > the indirect branch instruction meaning that its target does not have to
> > > start with ENDBR?
> >
> > CET-IBT has loads of get-out clauses.? The NOTRACK prefix is one; the
> > legacy code bitmap (implicit NOTRACK for whole libraries) is another.
> >
> > And yes - the purpose of NOTRACK is to exempt a specific indirect branch
> > from checks.
> >
> > GCC can emit NOTRACK'd calls in some cases when e.g. the programmer
> > launders a function pointer through (void *), or when
> > __attribute__((no_cf_check)) is used explicitly.
> >
> >
> > Each of the get-out clauses has separate enable bits, as each of them
> > reduces security.? In this series, Linux sets MSR_S_CET.ENDBR_EN but
> > specifically does not set NOTRACK_EN, so NOTRACK prefixes will be
> > ignored and suffer #CP if encountered.
>
> Thanks for the explanation. I would be nice to include it somewhere so
> that it is not lost.
I'll add something to the Changelog. Thanks!