The patch description mentions ``__attribute__((no_safe_stack))`` which isn't 
the name used anymore.


================
Comment at: docs/SafeStack.rst:18
@@ +17,3 @@
+buffer overflows on the unsafe stack cannot be used to overwrite anything
+on the safe stack, which includes return addresses.
+
----------------
It would be good to document the limitations that this approach has.

Off the top of my head:
* The safe stack is merely hidden from attackers by being somewhere in the 
address space. That's nice, but can be predictable even on 64-bit address 
spaces because not all the bits are addressable, multiple threads each have 
their stack, ...
* This approach doesn't prevent an attacker from "imbalancing" the safe stack 
by say having just one call, and doing two rets (thereby returning to an 
address that wasn't meant as a return address).

================
Comment at: docs/SafeStack.rst:51
@@ +50,3 @@
+implementations for C/C++ (e.g., Oilpan in chromium/blink), which must be
+changed to look for the live pointers on both safe and unsafe stacks.
+
----------------
Does this have any security implications by making Oilpan spill the SafeStack 
address? I'm assuming that care must be taken not to do so!

================
Comment at: docs/SafeStack.rst:55
@@ +54,3 @@
+SafeStack, both statically and dynamically. One corner case that is not
+supported is using dlopen() to load a dynamic library that uses SafeStack into
+a program that is not compiled with SafeStack but uses threads.
----------------
Format ``dlopen()``

================
Comment at: docs/SafeStack.rst:97
@@ +96,3 @@
+attribute may be required for functions that make assumptions about the
+exact layout of their stack frames.
+
----------------
What implications does this attribute have on the overall system's security?

================
Comment at: docs/SafeStack.rst:109
@@ +108,3 @@
+This builtin function returns a pointer to the start of the unsafe stack of the
+current thread.
+
----------------
What happens when calling ``__builtin_frame_address`` with SafeStack?

http://reviews.llvm.org/D6095

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to