[PATCH] D20084: [sanitizer] Initial implementation of a Hardened Allocator

[Kostya Serebryany via llvm-commits]

kcc added a comment.

doc-only comments....


================
Comment at: docs/HardenedAllocator.rst:13
@@ +12,3 @@
+CombinedAllocator, which aims at providing additional mitigations against heap
+based vulnerabilities, while retaining good performances.
+
----------------
[not a native English speaker here]
performances or performance? 

================
Comment at: docs/HardenedAllocator.rst:15
@@ +14,3 @@
+
+The name "Scudo" has been retained from the initial implementation (Escudo
+meaning Shield in Spanish and Portuguese).
----------------
Sweet! 

================
Comment at: docs/HardenedAllocator.rst:54
@@ +53,3 @@
+any type of double fetches of information located in the header, and use stack
+copies of the header for this purpose.
+
----------------
stack copies? 
I would call them local copies, because there is a good change that they are not on the stack but on a register

================
Comment at: docs/HardenedAllocator.rst:60
@@ +59,3 @@
+to keep it aside for a while. Once a criterion is met, the delayed freelist is
+emptied, and the quarantined chunks are returned to the backend. This help
+mitigates use-after-free vulnerabilities to reducing the determinism of the
----------------
helps mitigate?

================
Comment at: docs/HardenedAllocator.rst:61
@@ +60,3 @@
+emptied, and the quarantined chunks are returned to the backend. This help
+mitigates use-after-free vulnerabilities to reducing the determinism of the
+allocation and deallocation patterns.
----------------
s/to/by ? 

================
Comment at: docs/HardenedAllocator.rst:84
@@ +83,3 @@
+Linking the static library to your project will likely require the use of the
+"whole-archive" linker flag (or equivalent) as we make of the .preinit_array
+section to initialize the allocator. Additional linker flags can be required
----------------
"make use"?

================
Comment at: docs/HardenedAllocator.rst:88
@@ +87,3 @@
+
+Your linked binary should now make use of the Scudo allocation and deallocation
+functions.
----------------
You mean, dynamic linker? 
Static linker (such as e.g. lld) can safely use it, right? 

================
Comment at: docs/HardenedAllocator.rst:98
@@ +97,3 @@
+
+- quarantine_size_mb (integer, defaults to -1): the size (in Mb) of quarantine
+  used to delay the actual deallocation of chunks. Lower value may reduce
----------------
What do negative values mean? 


http://reviews.llvm.org/D20084