[PATCH] D29341: [scudo] 32-bit quarantine sizes adjustments and bug fixes

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added inline comments.


================
Comment at: lib/scudo/scudo_flags.cpp:71
   if (f->QuarantineSizeMb < 0) {
-    const int DefaultQuarantineSizeMb = 64;
+    const int DefaultQuarantineSizeMb = FIRST_32_SECOND_64(16, 64);
     f->QuarantineSizeMb = DefaultQuarantineSizeMb;
----------------
alekseyshl wrote:
> So, scudo default quarantine size is less than asan's one? Is there a reason?
The main reason being that the additional memory footprint incurred by the allocator should remain acceptable.
The ASan default of 256MB is, in my opinion, somewhat crippling. We just want the chunks to not be available right away, but if an attacker has the ability to allocate 64MB to get his chunk back, then he'll likely be able to allocate 256MB as well (to be faire 64MB is arbitrary, it could be less, but it feels like a decent number).


================
Comment at: test/scudo/quarantine.cpp:61
+    if (found == false)
+      return 1;
   }
----------------
alekseyshl wrote:
> Wouldn't assert(CONDITION); be more debug friendly than if (!CONDITION) return 1; throughout? Should any of those tests break, to figure out what exactly went wrong, one will have to either step through the code or add printfs/asserts anyway.
Good point, I will change those.


https://reviews.llvm.org/D29341