diff --git a/libutils/StrongPointer.cpp b/libutils/StrongPointer.cpp index ba52502ec..ef467238a 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 9cd7c75fd..07dd3f1cd 100644 --- a/libutils/include/utils/StrongPointer.h +++ b/libutils/include/utils/StrongPointer.h @@ -122,26 +122,54 @@ public: return o != *this; } -private: +private: template friend class sp; template 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() to allocate an object and wrap the resulting pointer safely +// without checking overhead. +template +void sp::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(MIN_PAGE_SIZE - 1); + uintptr_t my_frame_address = + reinterpret_cast(__builtin_frame_address(0 /* this frame */)); + if (((reinterpret_cast(ptr) ^ my_frame_address) & MIN_PAGE_MASK) == 0) { + sp_report_stack_pointer(); + } +} + template sp::sp(T* other) : m_ptr(other) { - if (other) + if (other) { + check_not_on_stack(other); other->incStrong(this); + } } template @@ -159,8 +187,10 @@ sp::sp(sp&& other) noexcept : m_ptr(other.m_ptr) { template template sp::sp(U* other) : m_ptr(other) { - if (other) + if (other) { + check_not_on_stack(other); (static_cast(other))->incStrong(this); + } } template template @@ -207,7 +237,10 @@ sp& sp::operator=(sp&& other) noexcept { template sp& sp::operator =(T* other) { T* oldPtr(*const_cast(&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(&m_ptr)) sp_report_race(); m_ptr = other;