Merge "Add check to sp<> raw pointer constructor"
am: c532941e54
Change-Id: I10e6b0bc269709e19665f9bd2d5c0b0b2622700d
diff --git a/libutils/StrongPointer.cpp b/libutils/StrongPointer.cpp
index ba52502..ef46723 100644
--- a/libutils/StrongPointer.cpp
+++ b/libutils/StrongPointer.cpp
@@ -21,4 +21,7 @@
namespace android {
void sp_report_race() { LOG_ALWAYS_FATAL("sp<> assignment detected data race"); }
+
+void sp_report_stack_pointer() { LOG_ALWAYS_FATAL("sp<> constructed with stack pointer argument"); }
+
}
diff --git a/libutils/include/utils/StrongPointer.h b/libutils/include/utils/StrongPointer.h
index 9cd7c75..07dd3f1 100644
--- a/libutils/include/utils/StrongPointer.h
+++ b/libutils/include/utils/StrongPointer.h
@@ -122,26 +122,54 @@
return o != *this;
}
-private:
+private:
template<typename Y> friend class sp;
template<typename Y> friend class wp;
void set_pointer(T* ptr);
+ static inline void check_not_on_stack(const void* ptr);
T* m_ptr;
};
-// For code size reasons, we do not want this inlined or templated.
+// For code size reasons, we do not want these inlined or templated.
void sp_report_race();
+void sp_report_stack_pointer();
#undef COMPARE
// ---------------------------------------------------------------------------
// No user serviceable parts below here.
+// Check whether address is definitely on the calling stack. We actually check whether it is on
+// the same 4K page as the frame pointer.
+//
+// Assumptions:
+// - Pages are never smaller than 4K (MIN_PAGE_SIZE)
+// - Malloced memory never shares a page with a stack.
+//
+// It does not appear safe to broaden this check to include adjacent pages; apparently this code
+// is used in environments where there may not be a guard page below (at higher addresses than)
+// the bottom of the stack.
+//
+// TODO: Consider adding make_sp<T>() to allocate an object and wrap the resulting pointer safely
+// without checking overhead.
+template <typename T>
+void sp<T>::check_not_on_stack(const void* ptr) {
+ static constexpr int MIN_PAGE_SIZE = 0x1000; // 4K. Safer than including sys/user.h.
+ static constexpr uintptr_t MIN_PAGE_MASK = ~static_cast<uintptr_t>(MIN_PAGE_SIZE - 1);
+ uintptr_t my_frame_address =
+ reinterpret_cast<uintptr_t>(__builtin_frame_address(0 /* this frame */));
+ if (((reinterpret_cast<uintptr_t>(ptr) ^ my_frame_address) & MIN_PAGE_MASK) == 0) {
+ sp_report_stack_pointer();
+ }
+}
+
template<typename T>
sp<T>::sp(T* other)
: m_ptr(other) {
- if (other)
+ if (other) {
+ check_not_on_stack(other);
other->incStrong(this);
+ }
}
template<typename T>
@@ -159,8 +187,10 @@
template<typename T> template<typename U>
sp<T>::sp(U* other)
: m_ptr(other) {
- if (other)
+ if (other) {
+ check_not_on_stack(other);
(static_cast<T*>(other))->incStrong(this);
+ }
}
template<typename T> template<typename U>
@@ -207,7 +237,10 @@
template<typename T>
sp<T>& sp<T>::operator =(T* other) {
T* oldPtr(*const_cast<T* volatile*>(&m_ptr));
- if (other) other->incStrong(this);
+ if (other) {
+ check_not_on_stack(other);
+ other->incStrong(this);
+ }
if (oldPtr) oldPtr->decStrong(this);
if (oldPtr != *const_cast<T* volatile*>(&m_ptr)) sp_report_race();
m_ptr = other;