From ce0f3201d1cf63c19bd1a47e5633023a5e683799 Mon Sep 17 00:00:00 2001 From: Sim Sun Date: Thu, 16 Apr 2020 04:02:59 -0700 Subject: [PATCH] Fix dangling pointer issue in LocalUpdatbleMaps Libunwindstack would remove duplicated items and update the `prev_map` during reparsing `/proc/self/maps`. But we leave `prev_real_map` pointing toward a MapInfo that will be deleted soon. It will cause a dangling pointer issue. Add new tests to cover this dangling pointer issue. Bug: 155511785 Test: libunwindstack_test Change-Id: I62e1b97bcb73f07e9349671f0b758f5ec9de16c0 (cherry picked from commit a7a194beb4a28f2da3d5098d1258d7284cca2b13) --- libunwindstack/Maps.cpp | 2 + .../tests/LocalUpdatableMapsTest.cpp | 99 +++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/libunwindstack/Maps.cpp b/libunwindstack/Maps.cpp index 8f49ad975..670d9046f 100644 --- a/libunwindstack/Maps.cpp +++ b/libunwindstack/Maps.cpp @@ -159,6 +159,8 @@ bool LocalUpdatableMaps::Reparse() { search_map_idx = old_map_idx + 1; if (new_map_idx + 1 < maps_.size()) { maps_[new_map_idx + 1]->prev_map = info.get(); + maps_[new_map_idx + 1]->prev_real_map = + info->IsBlank() ? info->prev_real_map : info.get(); } maps_[new_map_idx] = nullptr; total_entries--; diff --git a/libunwindstack/tests/LocalUpdatableMapsTest.cpp b/libunwindstack/tests/LocalUpdatableMapsTest.cpp index b816b9a83..99afb0b23 100644 --- a/libunwindstack/tests/LocalUpdatableMapsTest.cpp +++ b/libunwindstack/tests/LocalUpdatableMapsTest.cpp @@ -271,4 +271,103 @@ TEST_F(LocalUpdatableMapsTest, all_new_maps) { EXPECT_TRUE(map_info->name.empty()); } +TEST_F(LocalUpdatableMapsTest, add_map_prev_name_updated) { + TemporaryFile tf; + ASSERT_TRUE( + android::base::WriteStringToFile("3000-4000 rwxp 00000 00:00 0\n" + "8000-9000 r-xp 00000 00:00 0\n" + "9000-a000 r-xp 00000 00:00 0\n", + tf.path)); + + maps_.TestSetMapsFile(tf.path); + ASSERT_TRUE(maps_.Reparse()); + ASSERT_EQ(3U, maps_.Total()); + + MapInfo* map_info = maps_.Get(2); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0x9000U, map_info->start); + EXPECT_EQ(0xA000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, map_info->flags); + EXPECT_TRUE(map_info->name.empty()); + EXPECT_EQ(maps_.Get(1), map_info->prev_map); +} + +TEST_F(LocalUpdatableMapsTest, add_map_prev_real_name_updated) { + TemporaryFile tf; + ASSERT_TRUE( + android::base::WriteStringToFile("3000-4000 r-xp 00000 00:00 0 /fake/lib.so\n" + "4000-5000 ---p 00000 00:00 0\n" + "7000-8000 r-xp 00000 00:00 0 /fake/lib1.so\n" + "8000-9000 ---p 00000 00:00 0\n", + tf.path)); + + maps_.TestSetMapsFile(tf.path); + ASSERT_TRUE(maps_.Reparse()); + ASSERT_EQ(4U, maps_.Total()); + + MapInfo* map_info = maps_.Get(2); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0x7000U, map_info->start); + EXPECT_EQ(0x8000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, map_info->flags); + EXPECT_EQ(maps_.Get(0), map_info->prev_real_map); + EXPECT_EQ(maps_.Get(1), map_info->prev_map); + EXPECT_EQ("/fake/lib1.so", map_info->name); + + map_info = maps_.Get(3); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0x8000U, map_info->start); + EXPECT_EQ(0x9000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_TRUE(map_info->IsBlank()); + EXPECT_EQ(maps_.Get(2), map_info->prev_real_map); + EXPECT_EQ(maps_.Get(2), map_info->prev_map); + EXPECT_TRUE(map_info->name.empty()); + + ASSERT_TRUE( + android::base::WriteStringToFile("3000-4000 r-xp 00000 00:00 0 /fake/lib.so\n" + "4000-5000 ---p 00000 00:00 0\n" + "7000-8000 r-xp 00000 00:00 0 /fake/lib1.so\n" + "8000-9000 ---p 00000 00:00 0\n" + "9000-a000 r-xp 00000 00:00 0 /fake/lib2.so\n" + "a000-b000 r-xp 00000 00:00 0 /fake/lib3.so\n", + tf.path)); + + maps_.TestSetMapsFile(tf.path); + ASSERT_TRUE(maps_.Reparse()); + ASSERT_EQ(6U, maps_.Total()); + + map_info = maps_.Get(2); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0x7000U, map_info->start); + EXPECT_EQ(0x8000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, map_info->flags); + EXPECT_EQ("/fake/lib1.so", map_info->name); + EXPECT_EQ(maps_.Get(1), map_info->prev_map); + EXPECT_EQ(maps_.Get(0), map_info->prev_real_map); + + map_info = maps_.Get(4); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0x9000U, map_info->start); + EXPECT_EQ(0xA000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, map_info->flags); + EXPECT_EQ("/fake/lib2.so", map_info->name); + EXPECT_EQ(maps_.Get(3), map_info->prev_map); + EXPECT_EQ(maps_.Get(2), map_info->prev_real_map); + + map_info = maps_.Get(5); + ASSERT_TRUE(map_info != nullptr); + EXPECT_EQ(0xA000U, map_info->start); + EXPECT_EQ(0xB000U, map_info->end); + EXPECT_EQ(0U, map_info->offset); + EXPECT_EQ(PROT_READ | PROT_EXEC, map_info->flags); + EXPECT_EQ("/fake/lib3.so", map_info->name); + EXPECT_EQ(maps_.Get(4), map_info->prev_map); + EXPECT_EQ(maps_.Get(4), map_info->prev_real_map); +} + } // namespace unwindstack