From cc205131b021ebded854958973f445ed121da1b8 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 16 May 2017 14:01:47 -0700 Subject: Introduce CFX_UnownedPtr to detect lifetime inversion issues. There are places where an object "child" has a raw pointer back to object "owner" with the understanding that owner will always outlive child. Violating this constraint can lead to use after free, but this requires finding two paths: one that frees the objects in the wrong order, and one that uses the object after the free. The purpose of this patch is to detect the constraint violation even when the second path is not hit. We create a template that is used in place of TYPE*. It's dtor, when a memory tool is present, goes out and probes the first byte of the object to which it points. Used in "child", this allows the memory tool to prove that the "owner" is still alive at the time the child is destroyed, and hence the constraint is never violated. Change-Id: I2a6d696d51dda4a79ee2f00a6752965e058a6417 Reviewed-on: https://pdfium-review.googlesource.com/5475 Commit-Queue: Tom Sepez Reviewed-by: dsinclair Reviewed-by: Lei Zhang --- core/fxcrt/cfx_unowned_ptr.h | 64 ++++++++++++++++++++++++ core/fxcrt/cfx_unowned_ptr_unittest.cpp | 86 +++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 core/fxcrt/cfx_unowned_ptr.h create mode 100644 core/fxcrt/cfx_unowned_ptr_unittest.cpp (limited to 'core/fxcrt') diff --git a/core/fxcrt/cfx_unowned_ptr.h b/core/fxcrt/cfx_unowned_ptr.h new file mode 100644 index 0000000000..11f4e8e38b --- /dev/null +++ b/core/fxcrt/cfx_unowned_ptr.h @@ -0,0 +1,64 @@ +// Copyright 2017 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CORE_FXCRT_CFX_UNOWNED_PTR_H_ +#define CORE_FXCRT_CFX_UNOWNED_PTR_H_ + +#include +#include +#include +#include + +#include "core/fxcrt/fx_memory.h" + +template +class CFX_UnownedPtr { + public: + CFX_UnownedPtr() {} + CFX_UnownedPtr(const CFX_UnownedPtr& that) : CFX_UnownedPtr(that.Get()) {} + + template + explicit CFX_UnownedPtr(U* pObj) : m_pObj(pObj) {} + + // Deliberately implicit to allow returning nullptrs. + // NOLINTNEXTLINE(runtime/explicit) + CFX_UnownedPtr(std::nullptr_t ptr) {} + +#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR) + ~CFX_UnownedPtr() { + if (m_pObj) + reinterpret_cast(m_pObj)[0]; + } +#endif + + CFX_UnownedPtr& operator=(T* that) { + m_pObj = that; + return *this; + } + CFX_UnownedPtr& operator=(const CFX_UnownedPtr& that) { + if (*this != that) + m_pObj = that.Get(); + return *this; + } + + bool operator==(const CFX_UnownedPtr& that) const { + return Get() == that.Get(); + } + bool operator==(const T* that) const { return Get() == that; } + bool operator!=(const CFX_UnownedPtr& that) const { return !(*this == that); } + bool operator!=(const T* that) const { return !(*this == that); } + bool operator<(const CFX_UnownedPtr& that) const { + return std::less()(Get(), that.Get()); + } + + T* Get() const { return m_pObj; } + explicit operator bool() const { return !!m_pObj; } + T& operator*() const { return *m_pObj; } + T* operator->() const { return m_pObj; } + + private: + T* m_pObj = nullptr; +}; + +#endif // CORE_FXCRT_CFX_UNOWNED_PTR_H_ diff --git a/core/fxcrt/cfx_unowned_ptr_unittest.cpp b/core/fxcrt/cfx_unowned_ptr_unittest.cpp new file mode 100644 index 0000000000..7d29faedf5 --- /dev/null +++ b/core/fxcrt/cfx_unowned_ptr_unittest.cpp @@ -0,0 +1,86 @@ +// Copyright 2017 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/cfx_unowned_ptr.h" + +#include +#include + +#include "testing/fx_string_testhelpers.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class Clink { + public: + CFX_UnownedPtr next_ = nullptr; +}; + +void DeleteDangling() { + Clink* ptr1 = new Clink(); + Clink* ptr2 = new Clink(); + ptr2->next_ = ptr1; + delete ptr1; + delete ptr2; +} + +} // namespace + +TEST(fxcrt, UnownedPtrOk) { + Clink* ptr1 = new Clink(); + Clink* ptr2 = new Clink(); + ptr2->next_ = ptr1; + delete ptr2; + delete ptr1; +} + +TEST(fxcrt, UnownedPtrNotOk) { +#if defined(MEMORY_TOOL_REPLACES_ALLOCATOR) + EXPECT_DEATH(DeleteDangling(), ""); +#else + DeleteDangling(); +#endif +} + +TEST(fxcrt, OperatorEQ) { + int foo; + CFX_UnownedPtr ptr1; + EXPECT_TRUE(ptr1 == ptr1); + + CFX_UnownedPtr ptr2; + EXPECT_TRUE(ptr1 == ptr2); + + CFX_UnownedPtr ptr3(&foo); + EXPECT_TRUE(ptr3 == &foo); + EXPECT_FALSE(ptr1 == ptr3); + + ptr1 = &foo; + EXPECT_TRUE(ptr1 == ptr3); +} + +TEST(fxcrt, OperatorNE) { + int foo; + CFX_UnownedPtr ptr1; + EXPECT_FALSE(ptr1 != ptr1); + + CFX_UnownedPtr ptr2; + EXPECT_FALSE(ptr1 != ptr2); + + CFX_UnownedPtr ptr3(&foo); + EXPECT_FALSE(ptr3 != &foo); + EXPECT_TRUE(ptr1 != ptr3); + + ptr1 = &foo; + EXPECT_FALSE(ptr1 != ptr3); +} + +TEST(fxcrt, OperatorLT) { + int foos[2]; + CFX_UnownedPtr ptr1(&foos[0]); + CFX_UnownedPtr ptr2(&foos[1]); + + EXPECT_FALSE(ptr1 < ptr1); + EXPECT_TRUE(ptr1 < ptr2); + EXPECT_FALSE(ptr2 < ptr1); +} -- cgit v1.2.3