[PATCH] D77283: scudo: Add support for diagnosing memory errors when memory tagging is enabled.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 7 18:00:18 PDT 2020


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


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:744
+                           const char *MemoryTags, uintptr_t MemoryAddr,
+                           size_t MemorySize) {
+    *ErrorInfo = {};
----------------
hctim wrote:
> Just adding a tracking tag here - please LMK if you'd like me to write the fuzz target/oss-fuzz integration for this function. For now, I'm confident the following call will crash in `findNearestBlock` is called:
> 
> ```
> scudo_error_info ErrorInfo;
> const char EmptyBuffer[1] = {};
> getErrorInfo(&ErrorInfo, 0, EmptyBuffer, EmptyBuffer, EmptyBuffer, EmptyBuffer, 0, 1);
> ```
> 
> This function should pretty much take any sort of garbage - the result can be UB (IMO all entries of ErrorInfo should be nullptr or the function should return `false` on bad data), but it shouldn't crash. Worth validating the structs like Chromium GWP-ASan:  https://source.chromium.org/chromium/chromium/src/+/master:components/gwp_asan/common/allocator_state.cc;drc=28442cacc3be1a7d05a898aba663025a143095ac;bpv=1;bpt=1;l=64?originalUrl=https:%2F%2Fcs.chromium.org%2F
That might crash because `DepotPtr` and `RegionInfoPtr` aren't pointing to buffers that are large enough. I think we can trust the analyzing process to pass buffers that have appropriate sizes (e.g. `MemorySize` argument or `__scudo_*_size()` return value). The contents of the buffers themselves are another matter, of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77283





More information about the llvm-commits mailing list