[PATCH] D70762: scudo: Add initial memory tagging support.

Wed Nov 27 12:13:23 PST 2019

pcc marked an inline comment as done.
pcc added inline comments.

================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:247
+      // faulting potentially large numbers of pages for large secondary
+      // allocations. We assume that guard pages are enough to protect these
+      // allocations.
----------------
hctim wrote:
> In Chromium, ~11% of bugs are nonlinear (as determined with `Heap-buffer-*flow READ|WRITE {*}` over `Heap-buffer-*flow` with a fixed deterministic size).
> 
> The fixed size classes only go up to 24-byte allocations, so anything `24 < x <= [a page]` also land in this bucket - but we're also not counting wild SEGVs or UBSan errors that allow for attacker-controlled offsets...
> 
> I think it worth it to have a tagged secondary - although I underderstand there's some performance implications of this. Maybe guarded behind a runtime flag?
I'm not sure where you got the number 24 from. On Android we set MaxSizeLog to 17:
http://llvm-cs.pcc.me.uk/projects/compiler-rt/lib/scudo/standalone/size_class_map.h#143
so any allocations <= 2^17 bytes will use the primary allocator.

Tagged secondary would be nice, but I think I'd prefer to do it in a different way. Specifically, we might consider asking the kernel folks for a way to set the "background tag" of a mapping, so that any faulted pages in the mapping get the background tag. That way, we don't pay the cost of faulting up front.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762