[PATCH] D119296: KCFI sanitizer

[Joao Moreira via Phabricator via llvm-commits]

joaomoreira added inline comments.


================
Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:121
+    if (N == Value)
+      return ~Value;
+  }
----------------
samitolvanen wrote:
> joaomoreira wrote:
> > Can we use another constant blinding scheme, such as a Value++ or anything else? This way, we would prevent endbrs from being emitted in the indirect branch guards too.
> > 
> > Since we are using Value (prologue) and ~Value (caller/guard) for doing the checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise ENDBR will be emitted in the ibranch guards.
> > Can we use another constant blinding scheme, such as a Value++ or anything else? This way, we would prevent endbrs from being emitted in the indirect branch guards too.
> >
> > Since we are using Value (prologue) and ~Value (caller/guard) for doing the checks, we also need to check if ~ENDBR was picked as a KCFIType, otherwise ENDBR will be emitted in the ibranch guards.
> 
> I don't mind changing this to `Value + 1`, but that actually doesn't change anything because we emit `-Value` in indirect call checks, not `~Value`. Therefore, using `~Value` works equally well here.
> 
> Specifically, this code currently emits `~Value`in the preamble and `-(~Value) == Value + 1` in the indirect call check. Switching to `Value + 1` simply reverses the order; we'll emit `Value + 1` in the preamble and `-(Value + 1) == ~Value` in the indirect call check.
> 
> However, you are right that we also need to avoid `-ENDBR` in this function. I'll fix that and clarify the comment.
Oops, got confused with the operands, tks for clearing it up. Otherwise, changes LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119296/new/

https://reviews.llvm.org/D119296