Merge "libsnapshot: fix incorrect assumption of cow images"
diff --git a/adb/client/transport_mdns.cpp b/adb/client/transport_mdns.cpp
index 2b6aa7c..c9993b7 100644
--- a/adb/client/transport_mdns.cpp
+++ b/adb/client/transport_mdns.cpp
@@ -144,7 +144,7 @@
         return initialized_;
     }
 
-    virtual ~AsyncServiceRef() {
+    void DestroyServiceRef() {
         if (!initialized_) {
             return;
         }
@@ -152,9 +152,13 @@
         // Order matters here! Must destroy the fdevent first since it has a
         // reference to |sdRef_|.
         fdevent_destroy(fde_);
+        D("DNSServiceRefDeallocate(sdRef=%p)", sdRef_);
         DNSServiceRefDeallocate(sdRef_);
+        initialized_ = false;
     }
 
+    virtual ~AsyncServiceRef() { DestroyServiceRef(); }
+
   protected:
     DNSServiceRef sdRef_;
 
@@ -203,6 +207,7 @@
         if (ret != kDNSServiceErr_NoError) {
             D("Got %d from DNSServiceGetAddrInfo.", ret);
         } else {
+            D("DNSServiceGetAddrInfo(sdRef=%p, hosttarget=%s)", sdRef_, hosttarget);
             Initialize();
         }
 
@@ -223,7 +228,7 @@
         return true;
     }
 
-    void Connect(const sockaddr* address) {
+    bool AddToServiceRegistry(const sockaddr* address) {
         sa_family_ = address->sa_family;
 
         if (sa_family_ == AF_INET) {
@@ -234,13 +239,13 @@
             addr_format_ = "[%s]:%hu";
         } else {  // Should be impossible
             D("mDNS resolved non-IP address.");
-            return;
+            return false;
         }
 
         // Winsock version requires the const cast Because Microsoft.
         if (!inet_ntop(sa_family_, const_cast<void*>(ip_addr_data_), ip_addr_, sizeof(ip_addr_))) {
             D("Could not convert IP address to string.");
-            return;
+            return false;
         }
 
         // adb secure service needs to do something different from just
@@ -264,19 +269,32 @@
         }
 
         int adbSecureServiceType = serviceIndex();
+        ServiceRegistry* services = nullptr;
         switch (adbSecureServiceType) {
             case kADBTransportServiceRefIndex:
-                sAdbTransportServices->push_back(this);
+                services = sAdbTransportServices;
                 break;
             case kADBSecurePairingServiceRefIndex:
-                sAdbSecurePairingServices->push_back(this);
+                services = sAdbSecurePairingServices;
                 break;
             case kADBSecureConnectServiceRefIndex:
-                sAdbSecureConnectServices->push_back(this);
+                services = sAdbSecureConnectServices;
                 break;
             default:
-                break;
+                LOG(WARNING) << "No registry available for reg_type=[" << regType_ << "]";
+                return false;
         }
+
+        if (!services->empty()) {
+            // Remove the previous resolved service, if any.
+            services->erase(std::remove_if(services->begin(), services->end(),
+                                           [&](std::unique_ptr<ResolvedService>& service) {
+                                               return (serviceName_ == service->serviceName());
+                                           }));
+        }
+        services->push_back(std::unique_ptr<ResolvedService>(this));
+
+        return true;
     }
 
     int serviceIndex() const { return adb_DNSServiceIndexByName(regType_.c_str()); }
@@ -291,7 +309,7 @@
 
     uint16_t port() const { return port_; }
 
-    using ServiceRegistry = std::vector<ResolvedService*>;
+    using ServiceRegistry = std::vector<std::unique_ptr<ResolvedService>>;
 
     // unencrypted tcp connections
     static ServiceRegistry* sAdbTransportServices;
@@ -321,13 +339,13 @@
 };
 
 // static
-std::vector<ResolvedService*>* ResolvedService::sAdbTransportServices = NULL;
+ResolvedService::ServiceRegistry* ResolvedService::sAdbTransportServices = NULL;
 
 // static
-std::vector<ResolvedService*>* ResolvedService::sAdbSecurePairingServices = NULL;
+ResolvedService::ServiceRegistry* ResolvedService::sAdbSecurePairingServices = NULL;
 
 // static
-std::vector<ResolvedService*>* ResolvedService::sAdbSecureConnectServices = NULL;
+ResolvedService::ServiceRegistry* ResolvedService::sAdbSecureConnectServices = NULL;
 
 // static
 void ResolvedService::initAdbServiceRegistries() {
@@ -348,7 +366,7 @@
                                      adb_secure_foreach_service_callback cb) {
     initAdbServiceRegistries();
 
-    for (auto service : services) {
+    for (const auto& service : services) {
         auto service_name = service->serviceName();
         auto reg_type = service->regType();
         auto ip = service->ipAddress();
@@ -366,7 +384,7 @@
 bool ResolvedService::connectByServiceName(const ServiceRegistry& services,
                                            const std::string& service_name) {
     initAdbServiceRegistries();
-    for (auto service : services) {
+    for (const auto& service : services) {
         if (service_name == service->serviceName()) {
             D("Got service_name match [%s]", service->serviceName().c_str());
             return service->ConnectSecureWifiDevice();
@@ -393,23 +411,28 @@
                                                  service_name);
 }
 
-static void DNSSD_API register_service_ip(DNSServiceRef /*sdRef*/,
-                                          DNSServiceFlags /*flags*/,
+static void DNSSD_API register_service_ip(DNSServiceRef sdRef, DNSServiceFlags flags,
                                           uint32_t /*interfaceIndex*/,
-                                          DNSServiceErrorType /*errorCode*/,
-                                          const char* /*hostname*/,
-                                          const sockaddr* address,
-                                          uint32_t /*ttl*/,
-                                          void* context) {
-    D("Got IP for service.");
+                                          DNSServiceErrorType errorCode, const char* hostname,
+                                          const sockaddr* address, uint32_t ttl, void* context) {
+    D("%s: sdRef=%p flags=0x%08x errorCode=%u ttl=%u", __func__, sdRef, flags, errorCode, ttl);
     std::unique_ptr<ResolvedService> data(
         reinterpret_cast<ResolvedService*>(context));
-    data->Connect(address);
+    // Only resolve the address once. If the address or port changes, we'll just get another
+    // registration.
+    data->DestroyServiceRef();
 
-    // For ADB Secure services, keep those ResolvedService's around
-    // for later processing with secure connection establishment.
-    if (data->serviceIndex() != kADBTransportServiceRefIndex) {
-        data.release();
+    if (errorCode != kDNSServiceErr_NoError) {
+        D("Got error while looking up ipaddr [%u]", errorCode);
+        return;
+    }
+
+    if (flags & kDNSServiceFlagsAdd) {
+        D("Resolved IP address for [%s]. Adding to service registry.", hostname);
+        auto* ptr = data.release();
+        if (!ptr->AddToServiceRegistry(address)) {
+            data.reset(ptr);
+        }
     }
 }
 
@@ -459,6 +482,7 @@
 };
 
 static void adb_RemoveDNSService(const char* regType, const char* serviceName) {
+    D("%s: regType=[%s] serviceName=[%s]", __func__, regType, serviceName);
     int index = adb_DNSServiceIndexByName(regType);
     ResolvedService::ServiceRegistry* services;
     switch (index) {
@@ -475,10 +499,15 @@
             return;
     }
 
+    if (services->empty()) {
+        return;
+    }
+
     std::string sName(serviceName);
-    services->erase(std::remove_if(
-            services->begin(), services->end(),
-            [&sName](ResolvedService* service) { return (sName == service->serviceName()); }));
+    services->erase(std::remove_if(services->begin(), services->end(),
+                                   [&sName](std::unique_ptr<ResolvedService>& service) {
+                                       return (sName == service->serviceName());
+                                   }));
 }
 
 // Returns the version the device wanted to advertise,
diff --git a/adb/test_adb.py b/adb/test_adb.py
index 03bdcbd..9912f11 100755
--- a/adb/test_adb.py
+++ b/adb/test_adb.py
@@ -659,14 +659,14 @@
                 print(f"Registering {serv_instance}.{serv_type} ...")
                 with zeroconf_register_service(zc, service_info) as info:
                     """Give adb some time to register the service"""
-                    time.sleep(0.25)
+                    time.sleep(1)
                     print(f"services={_mdns_services(server_port)}")
                     self.assertTrue(any((serv_instance in line and serv_type in line)
                         for line in _mdns_services(server_port)))
 
                 """Give adb some time to unregister the service"""
                 print("Unregistering mdns service...")
-                time.sleep(0.25)
+                time.sleep(1)
                 print(f"services={_mdns_services(server_port)}")
                 self.assertFalse(any((serv_instance in line and serv_type in line)
                     for line in _mdns_services(server_port)))
diff --git a/fs_mgr/libfiemap/image_test.cpp b/fs_mgr/libfiemap/image_test.cpp
index 5388b44..6663391 100644
--- a/fs_mgr/libfiemap/image_test.cpp
+++ b/fs_mgr/libfiemap/image_test.cpp
@@ -131,132 +131,6 @@
     ASSERT_TRUE(manager_->UnmapImageDevice(base_name_));
 }
 
-// This fixture is for tests against a simulated device environment. Rather
-// than use /data, we create an image and then layer a new filesystem within
-// it. Each test then decides how to mount and create layered images. This
-// allows us to test FBE vs FDE configurations.
-class ImageTest : public ::testing::Test {
-  public:
-    ImageTest() : dm_(DeviceMapper::Instance()) {}
-
-    void SetUp() override {
-        manager_ = ImageManager::Open(kMetadataPath, gDataPath);
-        ASSERT_NE(manager_, nullptr);
-
-        manager_->set_partition_opener(std::make_unique<TestPartitionOpener>());
-
-        submanager_ = ImageManager::Open(kMetadataPath + "/mnt"s, gDataPath + "/mnt"s);
-        ASSERT_NE(submanager_, nullptr);
-
-        submanager_->set_partition_opener(std::make_unique<TestPartitionOpener>());
-
-        // Ensure that metadata is cleared in between runs.
-        submanager_->RemoveAllImages();
-        manager_->RemoveAllImages();
-
-        const ::testing::TestInfo* tinfo = ::testing::UnitTest::GetInstance()->current_test_info();
-        base_name_ = tinfo->name();
-        test_image_name_ = base_name_ + "-base";
-        wrapper_device_name_ = base_name_ + "-wrapper";
-
-        ASSERT_TRUE(manager_->CreateBackingImage(base_name_, kTestImageSize * 16, false, nullptr));
-        ASSERT_TRUE(manager_->MapImageDevice(base_name_, 5s, &base_device_));
-    }
-
-    void TearDown() override {
-        submanager_->UnmapImageDevice(test_image_name_);
-        umount(gDataMountPath.c_str());
-        dm_.DeleteDeviceIfExists(wrapper_device_name_);
-        manager_->UnmapImageDevice(base_name_);
-        manager_->DeleteBackingImage(base_name_);
-    }
-
-  protected:
-    bool DoFormat(const std::string& device) {
-        // clang-format off
-        std::vector<std::string> mkfs_args = {
-            "/system/bin/mke2fs",
-            "-F",
-            "-b 4096",
-            "-t ext4",
-            "-m 0",
-            "-O has_journal",
-            device,
-            ">/dev/null",
-            "2>/dev/null",
-            "</dev/null",
-        };
-        // clang-format on
-        auto command = android::base::Join(mkfs_args, " ");
-        return system(command.c_str()) == 0;
-    }
-
-    std::unique_ptr<ImageManager> manager_;
-    std::unique_ptr<ImageManager> submanager_;
-
-    DeviceMapper& dm_;
-    std::string base_name_;
-    std::string base_device_;
-    std::string test_image_name_;
-    std::string wrapper_device_name_;
-};
-
-TEST_F(ImageTest, DirectMount) {
-    ASSERT_TRUE(DoFormat(base_device_));
-    ASSERT_EQ(mount(base_device_.c_str(), gDataMountPath.c_str(), "ext4", 0, nullptr), 0);
-    ASSERT_TRUE(submanager_->CreateBackingImage(test_image_name_, kTestImageSize, false, nullptr));
-
-    std::string path;
-    ASSERT_TRUE(submanager_->MapImageDevice(test_image_name_, 5s, &path));
-    ASSERT_TRUE(android::base::StartsWith(path, "/dev/block/loop"));
-}
-
-TEST_F(ImageTest, IndirectMount) {
-#ifdef SKIP_TEST_IN_PRESUBMIT
-    GTEST_SKIP() << "WIP failure b/148874852";
-#endif
-    // Create a simple wrapper around the base device that we'll mount from
-    // instead. This will simulate the code paths for dm-crypt/default-key/bow
-    // and force us to use device-mapper rather than loop devices.
-    uint64_t device_size = 0;
-    {
-        unique_fd fd(open(base_device_.c_str(), O_RDWR | O_CLOEXEC));
-        ASSERT_GE(fd, 0);
-        device_size = get_block_device_size(fd);
-        ASSERT_EQ(device_size, kTestImageSize * 16);
-    }
-    uint64_t num_sectors = device_size / 512;
-
-    auto& dm = DeviceMapper::Instance();
-
-    DmTable table;
-    table.Emplace<DmTargetLinear>(0, num_sectors, base_device_, 0);
-    ASSERT_TRUE(dm.CreateDevice(wrapper_device_name_, table));
-
-    // Format and mount.
-    std::string wrapper_device;
-    ASSERT_TRUE(dm.GetDmDevicePathByName(wrapper_device_name_, &wrapper_device));
-    ASSERT_TRUE(WaitForFile(wrapper_device, 5s));
-    ASSERT_TRUE(DoFormat(wrapper_device));
-    ASSERT_EQ(mount(wrapper_device.c_str(), gDataMountPath.c_str(), "ext4", 0, nullptr), 0);
-
-    ASSERT_TRUE(submanager_->CreateBackingImage(test_image_name_, kTestImageSize, false, nullptr));
-
-    std::set<std::string> backing_devices;
-    auto init = [&](std::set<std::string> devices) -> bool {
-        backing_devices = std::move(devices);
-        return true;
-    };
-
-    std::string path;
-    ASSERT_TRUE(submanager_->MapImageDevice(test_image_name_, 5s, &path));
-    ASSERT_TRUE(android::base::StartsWith(path, "/dev/block/dm-"));
-    ASSERT_TRUE(submanager_->UnmapImageDevice(test_image_name_));
-    ASSERT_TRUE(submanager_->MapAllImages(init));
-    ASSERT_FALSE(backing_devices.empty());
-    ASSERT_TRUE(submanager_->UnmapImageDevice(test_image_name_));
-}
-
 bool Mkdir(const std::string& path) {
     if (mkdir(path.c_str(), 0700) && errno != EEXIST) {
         std::cerr << "Could not mkdir " << path << ": " << strerror(errno) << std::endl;
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index dc3b985..c37d70e 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -19,6 +19,7 @@
 #include <string.h>
 
 #include <algorithm>
+#include <limits>
 
 #include <android-base/unique_fd.h>
 
@@ -369,7 +370,10 @@
     }
 
     // Align the metadata size up to the nearest sector.
-    metadata_max_size = AlignTo(metadata_max_size, LP_SECTOR_SIZE);
+    if (!AlignTo(metadata_max_size, LP_SECTOR_SIZE, &metadata_max_size)) {
+        LERROR << "Max metadata size " << metadata_max_size << " is too large.";
+        return false;
+    }
 
     // Validate and build the block device list.
     uint32_t logical_block_size = 0;
@@ -401,10 +405,15 @@
         // untouched to be compatible code that looks for an MBR. Thus we
         // start counting free sectors at sector 1, not 0.
         uint64_t free_area_start = LP_SECTOR_SIZE;
+        bool ok;
         if (out.alignment) {
-            free_area_start = AlignTo(free_area_start, out.alignment);
+            ok = AlignTo(free_area_start, out.alignment, &free_area_start);
         } else {
-            free_area_start = AlignTo(free_area_start, logical_block_size);
+            ok = AlignTo(free_area_start, logical_block_size, &free_area_start);
+        }
+        if (!ok) {
+            LERROR << "Integer overflow computing free area start";
+            return false;
         }
         out.first_logical_sector = free_area_start / LP_SECTOR_SIZE;
 
@@ -441,10 +450,15 @@
 
     // Compute the first free sector, factoring in alignment.
     uint64_t free_area_start = total_reserved;
+    bool ok;
     if (super.alignment || super.alignment_offset) {
-        free_area_start = AlignTo(free_area_start, super.alignment);
+        ok = AlignTo(free_area_start, super.alignment, &free_area_start);
     } else {
-        free_area_start = AlignTo(free_area_start, logical_block_size);
+        ok = AlignTo(free_area_start, logical_block_size, &free_area_start);
+    }
+    if (!ok) {
+        LERROR << "Integer overflow computing free area start";
+        return false;
     }
     super.first_logical_sector = free_area_start / LP_SECTOR_SIZE;
 
@@ -544,7 +558,11 @@
         const Interval& current = extents[i];
         DCHECK(previous.device_index == current.device_index);
 
-        uint64_t aligned = AlignSector(block_devices_[current.device_index], previous.end);
+        uint64_t aligned;
+        if (!AlignSector(block_devices_[current.device_index], previous.end, &aligned)) {
+            LERROR << "Sector " << previous.end << " caused integer overflow.";
+            continue;
+        }
         if (aligned >= current.start) {
             // There is no gap between these two extents, try the next one.
             // Note that we check with >= instead of >, since alignment may
@@ -730,7 +748,10 @@
     // Choose an aligned sector for the midpoint. This could lead to one half
     // being slightly larger than the other, but this will not restrict the
     // size of partitions (it might lead to one extra extent if "B" overflows).
-    midpoint = AlignSector(super, midpoint);
+    if (!AlignSector(super, midpoint, &midpoint)) {
+        LERROR << "Unexpected integer overflow aligning midpoint " << midpoint;
+        return free_list;
+    }
 
     std::vector<Interval> first_half;
     std::vector<Interval> second_half;
@@ -768,7 +789,11 @@
     // If the sector ends where the next aligned chunk begins, then there's
     // no missing gap to try and allocate.
     const auto& block_device = block_devices_[extent->device_index()];
-    uint64_t next_aligned_sector = AlignSector(block_device, extent->end_sector());
+    uint64_t next_aligned_sector;
+    if (!AlignSector(block_device, extent->end_sector(), &next_aligned_sector)) {
+        LERROR << "Integer overflow aligning sector " << extent->end_sector();
+        return nullptr;
+    }
     if (extent->end_sector() == next_aligned_sector) {
         return nullptr;
     }
@@ -925,13 +950,19 @@
     return size;
 }
 
-uint64_t MetadataBuilder::AlignSector(const LpMetadataBlockDevice& block_device,
-                                      uint64_t sector) const {
+bool MetadataBuilder::AlignSector(const LpMetadataBlockDevice& block_device, uint64_t sector,
+                                  uint64_t* out) const {
     // Note: when reading alignment info from the Kernel, we don't assume it
     // is aligned to the sector size, so we round up to the nearest sector.
     uint64_t lba = sector * LP_SECTOR_SIZE;
-    uint64_t aligned = AlignTo(lba, block_device.alignment);
-    return AlignTo(aligned, LP_SECTOR_SIZE) / LP_SECTOR_SIZE;
+    if (!AlignTo(lba, block_device.alignment, out)) {
+        return false;
+    }
+    if (!AlignTo(*out, LP_SECTOR_SIZE, out)) {
+        return false;
+    }
+    *out /= LP_SECTOR_SIZE;
+    return true;
 }
 
 bool MetadataBuilder::FindBlockDeviceByName(const std::string& partition_name,
@@ -1005,7 +1036,12 @@
 bool MetadataBuilder::ResizePartition(Partition* partition, uint64_t requested_size,
                                       const std::vector<Interval>& free_region_hint) {
     // Align the space needed up to the nearest sector.
-    uint64_t aligned_size = AlignTo(requested_size, geometry_.logical_block_size);
+    uint64_t aligned_size;
+    if (!AlignTo(requested_size, geometry_.logical_block_size, &aligned_size)) {
+        LERROR << "Cannot resize partition " << partition->name() << " to " << requested_size
+               << " bytes; integer overflow.";
+        return false;
+    }
     uint64_t old_size = partition->size();
 
     if (!ValidatePartitionSizeChange(partition, old_size, aligned_size, false)) {
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 52a3217..1a3250a 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -228,8 +228,9 @@
         ASSERT_EQ(extent.target_type, LP_TARGET_TYPE_LINEAR);
         EXPECT_EQ(extent.num_sectors, 80);
 
+        uint64_t aligned_lba;
         uint64_t lba = extent.target_data * LP_SECTOR_SIZE;
-        uint64_t aligned_lba = AlignTo(lba, device_info.alignment);
+        ASSERT_TRUE(AlignTo(lba, device_info.alignment, &aligned_lba));
         EXPECT_EQ(lba, aligned_lba);
     }
 
@@ -1051,3 +1052,17 @@
     EXPECT_EQ(e2->physical_sector(), 3072);
     EXPECT_EQ(e2->end_sector(), 4197368);
 }
+
+TEST_F(BuilderTest, ResizeOverflow) {
+    BlockDeviceInfo super("super", 8_GiB, 786432, 229376, 4096);
+    std::vector<BlockDeviceInfo> block_devices = {super};
+
+    unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(block_devices, "super", 65536, 2);
+    ASSERT_NE(builder, nullptr);
+
+    ASSERT_TRUE(builder->AddGroup("group", 0));
+
+    Partition* p = builder->AddPartition("system", "default", 0);
+    ASSERT_NE(p, nullptr);
+    ASSERT_FALSE(builder->ResizePartition(p, 18446744073709551615ULL));
+}
diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h
index bd39150..732dbea 100644
--- a/fs_mgr/liblp/include/liblp/builder.h
+++ b/fs_mgr/liblp/include/liblp/builder.h
@@ -359,7 +359,7 @@
     bool GrowPartition(Partition* partition, uint64_t aligned_size,
                        const std::vector<Interval>& free_region_hint);
     void ShrinkPartition(Partition* partition, uint64_t aligned_size);
-    uint64_t AlignSector(const LpMetadataBlockDevice& device, uint64_t sector) const;
+    bool AlignSector(const LpMetadataBlockDevice& device, uint64_t sector, uint64_t* out) const;
     uint64_t TotalSizeOfGroup(PartitionGroup* group) const;
     bool UpdateBlockDeviceInfo(size_t index, const BlockDeviceInfo& info);
     bool FindBlockDeviceByName(const std::string& partition_name, uint32_t* index) const;
diff --git a/fs_mgr/liblp/utility.h b/fs_mgr/liblp/utility.h
index f210eaf..c4fe3ed 100644
--- a/fs_mgr/liblp/utility.h
+++ b/fs_mgr/liblp/utility.h
@@ -21,6 +21,7 @@
 #include <stdint.h>
 #include <sys/types.h>
 
+#include <limits>
 #include <string>
 #include <string_view>
 
@@ -66,16 +67,26 @@
 void SHA256(const void* data, size_t length, uint8_t out[32]);
 
 // Align |base| such that it is evenly divisible by |alignment|, which does not
-// have to be a power of two.
-constexpr uint64_t AlignTo(uint64_t base, uint32_t alignment) {
+// have to be a power of two. Return false on overflow.
+template <typename T>
+bool AlignTo(T base, uint32_t alignment, T* out) {
+    static_assert(std::numeric_limits<T>::is_integer);
+    static_assert(!std::numeric_limits<T>::is_signed);
     if (!alignment) {
-        return base;
+        *out = base;
+        return true;
     }
-    uint64_t remainder = base % alignment;
+    T remainder = base % alignment;
     if (remainder == 0) {
-        return base;
+        *out = base;
+        return true;
     }
-    return base + (alignment - remainder);
+    T to_add = alignment - remainder;
+    if (to_add > std::numeric_limits<T>::max() - base) {
+        return false;
+    }
+    *out = base + to_add;
+    return true;
 }
 
 // Update names from C++ strings.
diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp
index b64861d..fc90872 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <optional>
+
 #include <gtest/gtest.h>
 #include <liblp/builder.h>
 #include <liblp/liblp.h>
@@ -58,15 +60,28 @@
     EXPECT_EQ(GetBackupMetadataOffset(geometry, 0), backup_start + 16384 * 0);
 }
 
+std::optional<uint64_t> AlignTo(uint64_t base, uint32_t alignment) {
+    uint64_t r;
+    if (!AlignTo(base, alignment, &r)) {
+        return {};
+    }
+    return {r};
+}
+
 TEST(liblp, AlignTo) {
-    EXPECT_EQ(AlignTo(37, 0), 37);
-    EXPECT_EQ(AlignTo(1024, 1024), 1024);
-    EXPECT_EQ(AlignTo(555, 1024), 1024);
-    EXPECT_EQ(AlignTo(555, 1000), 1000);
-    EXPECT_EQ(AlignTo(0, 1024), 0);
-    EXPECT_EQ(AlignTo(54, 32), 64);
-    EXPECT_EQ(AlignTo(32, 32), 32);
-    EXPECT_EQ(AlignTo(17, 32), 32);
+    EXPECT_EQ(AlignTo(37, 0), std::optional<uint64_t>(37));
+    EXPECT_EQ(AlignTo(1024, 1024), std::optional<uint64_t>(1024));
+    EXPECT_EQ(AlignTo(555, 1024), std::optional<uint64_t>(1024));
+    EXPECT_EQ(AlignTo(555, 1000), std::optional<uint64_t>(1000));
+    EXPECT_EQ(AlignTo(0, 1024), std::optional<uint64_t>(0));
+    EXPECT_EQ(AlignTo(54, 32), std::optional<uint64_t>(64));
+    EXPECT_EQ(AlignTo(32, 32), std::optional<uint64_t>(32));
+    EXPECT_EQ(AlignTo(17, 32), std::optional<uint64_t>(32));
+
+    auto u32limit = std::numeric_limits<uint32_t>::max();
+    auto u64limit = std::numeric_limits<uint64_t>::max();
+    EXPECT_EQ(AlignTo(u64limit - u32limit + 1, u32limit), std::optional<uint64_t>{u64limit});
+    EXPECT_EQ(AlignTo(std::numeric_limits<uint64_t>::max(), 2), std::optional<uint64_t>{});
 }
 
 TEST(liblp, GetPartitionSlotSuffix) {
diff --git a/libunwindstack/Elf.cpp b/libunwindstack/Elf.cpp
index f01b092..286febc 100644
--- a/libunwindstack/Elf.cpp
+++ b/libunwindstack/Elf.cpp
@@ -124,6 +124,12 @@
     return false;
   }
 
+  if (arch() == ARCH_ARM64) {
+    // Tagged pointer after Android R would lead top byte to have random values
+    // https://source.android.com/devices/tech/debug/tagged-pointers
+    vaddr &= (1ULL << 56) - 1;
+  }
+
   // Check the .data section.
   uint64_t vaddr_start = interface_->data_vaddr_start();
   if (vaddr >= vaddr_start && vaddr < interface_->data_vaddr_end()) {
diff --git a/libunwindstack/tests/ElfFake.h b/libunwindstack/tests/ElfFake.h
index fc90dab..3b6cb80 100644
--- a/libunwindstack/tests/ElfFake.h
+++ b/libunwindstack/tests/ElfFake.h
@@ -55,6 +55,8 @@
 
   void FakeSetLoadBias(uint64_t load_bias) { load_bias_ = load_bias; }
 
+  void FakeSetArch(ArchEnum arch) { arch_ = arch; }
+
   void FakeSetInterface(ElfInterface* interface) { interface_.reset(interface); }
   void FakeSetGnuDebugdataInterface(ElfInterface* interface) {
     gnu_debugdata_interface_.reset(interface);
diff --git a/libunwindstack/tests/ElfTest.cpp b/libunwindstack/tests/ElfTest.cpp
index 1f3ed81..f0852a4 100644
--- a/libunwindstack/tests/ElfTest.cpp
+++ b/libunwindstack/tests/ElfTest.cpp
@@ -438,6 +438,48 @@
   EXPECT_EQ(0xc080U, offset);
 }
 
+TEST_F(ElfTest, get_global_vaddr_with_tagged_pointer) {
+  ElfFake elf(memory_);
+  elf.FakeSetValid(true);
+  elf.FakeSetArch(ARCH_ARM64);
+
+  ElfInterfaceMock* interface = new ElfInterfaceMock(memory_);
+  elf.FakeSetInterface(interface);
+  interface->MockSetDataVaddrStart(0x500);
+  interface->MockSetDataVaddrEnd(0x600);
+  interface->MockSetDataOffset(0xa000);
+
+  std::string global("something");
+  EXPECT_CALL(*interface, GetGlobalVariable(global, ::testing::_))
+      .WillOnce(::testing::DoAll(::testing::SetArgPointee<1>(0x8800000000000580),
+                                 ::testing::Return(true)));
+
+  uint64_t offset;
+  ASSERT_TRUE(elf.GetGlobalVariableOffset(global, &offset));
+  EXPECT_EQ(0xa080U, offset);
+}
+
+TEST_F(ElfTest, get_global_vaddr_without_tagged_pointer) {
+  ElfFake elf(memory_);
+  elf.FakeSetValid(true);
+  elf.FakeSetArch(ARCH_X86_64);
+
+  ElfInterfaceMock* interface = new ElfInterfaceMock(memory_);
+  elf.FakeSetInterface(interface);
+  interface->MockSetDataVaddrStart(0x8800000000000500);
+  interface->MockSetDataVaddrEnd(0x8800000000000600);
+  interface->MockSetDataOffset(0x880000000000a000);
+
+  std::string global("something");
+  EXPECT_CALL(*interface, GetGlobalVariable(global, ::testing::_))
+      .WillOnce(::testing::DoAll(::testing::SetArgPointee<1>(0x8800000000000580),
+                                 ::testing::Return(true)));
+
+  uint64_t offset;
+  ASSERT_TRUE(elf.GetGlobalVariableOffset(global, &offset));
+  EXPECT_EQ(0x880000000000a080U, offset);
+}
+
 TEST_F(ElfTest, is_valid_pc_elf_invalid) {
   ElfFake elf(memory_);
   elf.FakeSetValid(false);
diff --git a/logd/Android.bp b/logd/Android.bp
index 2663271..b6d30cd 100644
--- a/logd/Android.bp
+++ b/logd/Android.bp
@@ -36,9 +36,9 @@
         "CommandListener.cpp",
         "LogListener.cpp",
         "LogReader.cpp",
+        "LogReaderThread.cpp",
         "LogBuffer.cpp",
         "LogBufferElement.cpp",
-        "LogTimes.cpp",
         "LogStatistics.cpp",
         "LogWhiteBlackList.cpp",
         "libaudit.c",
diff --git a/logd/LogAudit.cpp b/logd/LogAudit.cpp
index d9cc0db..37067bd 100644
--- a/logd/LogAudit.cpp
+++ b/logd/LogAudit.cpp
@@ -277,7 +277,7 @@
                   : LOGGER_ENTRY_MAX_PAYLOAD;
     size_t message_len = str_len + sizeof(android_log_event_string_t);
 
-    log_mask_t notify = 0;
+    unsigned int notify = 0;
 
     if (events) {  // begin scope for event buffer
         uint32_t buffer[(message_len + sizeof(uint32_t) - 1) / sizeof(uint32_t)];
diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp
index a3e4e09..a7323e8 100644
--- a/logd/LogBuffer.cpp
+++ b/logd/LogBuffer.cpp
@@ -93,16 +93,16 @@
     }
 
     // Release any sleeping reader threads to dump their current content.
-    LogTimeEntry::wrlock();
+    LogReaderThread::wrlock();
 
     LastLogTimes::iterator times = mTimes.begin();
     while (times != mTimes.end()) {
-        LogTimeEntry* entry = times->get();
+        LogReaderThread* entry = times->get();
         entry->triggerReader_Locked();
         times++;
     }
 
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 }
 
 LogBuffer::LogBuffer(LastLogTimes* times, LogTags* tags, PruneList* prune)
@@ -579,14 +579,14 @@
 
 // If the selected reader is blocking our pruning progress, decide on
 // what kind of mitigation is necessary to unblock the situation.
-void LogBuffer::kickMe(LogTimeEntry* me, log_id_t id, unsigned long pruneRows) {
+void LogBuffer::kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows) {
     if (stats.sizes(id) > (2 * log_buffer_size(id))) {  // +100%
         // A misbehaving or slow reader has its connection
         // dropped if we hit too much memory pressure.
         android::prdebug("Kicking blocked reader, pid %d, from LogBuffer::kickMe()\n",
-                         me->mClient->getPid());
+                         me->client()->getPid());
         me->release_Locked();
-    } else if (me->mTimeout.tv_sec || me->mTimeout.tv_nsec) {
+    } else if (me->timeout().tv_sec || me->timeout().tv_nsec) {
         // Allow a blocked WRAP timeout reader to
         // trigger and start reporting the log data.
         me->triggerReader_Locked();
@@ -594,7 +594,7 @@
         // tell slow reader to skip entries to catch up
         android::prdebug(
                 "Skipping %lu entries from slow reader, pid %d, from LogBuffer::kickMe()\n",
-                pruneRows, me->mClient->getPid());
+                pruneRows, me->client()->getPid());
         me->triggerSkip_Locked(id, pruneRows);
     }
 }
@@ -647,20 +647,19 @@
 // LogBuffer::wrlock() must be held when this function is called.
 //
 bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) {
-    LogTimeEntry* oldest = nullptr;
+    LogReaderThread* oldest = nullptr;
     bool busy = false;
     bool clearAll = pruneRows == ULONG_MAX;
 
-    LogTimeEntry::rdlock();
+    LogReaderThread::rdlock();
 
     // Region locked?
     LastLogTimes::iterator times = mTimes.begin();
     while (times != mTimes.end()) {
-        LogTimeEntry* entry = times->get();
-        if (entry->isWatching(id) &&
-            (!oldest || (oldest->mStart > entry->mStart) ||
-             ((oldest->mStart == entry->mStart) &&
-              (entry->mTimeout.tv_sec || entry->mTimeout.tv_nsec)))) {
+        LogReaderThread* entry = times->get();
+        if (entry->IsWatching(id) && (!oldest || oldest->start() > entry->start() ||
+                                      (oldest->start() == entry->start() &&
+                                       (entry->timeout().tv_sec || entry->timeout().tv_nsec)))) {
             oldest = entry;
         }
         times++;
@@ -681,7 +680,7 @@
                 continue;
             }
 
-            if (oldest && oldest->mStart <= element->getSequence()) {
+            if (oldest && oldest->start() <= element->getSequence()) {
                 busy = true;
                 kickMe(oldest, id, pruneRows);
                 break;
@@ -692,7 +691,7 @@
                 break;
             }
         }
-        LogTimeEntry::unlock();
+        LogReaderThread::unlock();
         return busy;
     }
 
@@ -772,7 +771,7 @@
         while (it != mLogElements.end()) {
             LogBufferElement* element = *it;
 
-            if (oldest && oldest->mStart <= element->getSequence()) {
+            if (oldest && oldest->start() <= element->getSequence()) {
                 busy = true;
                 // Do not let chatty eliding trigger any reader mitigation
                 break;
@@ -914,7 +913,7 @@
             continue;
         }
 
-        if (oldest && oldest->mStart <= element->getSequence()) {
+        if (oldest && oldest->start() <= element->getSequence()) {
             busy = true;
             if (!whitelist) kickMe(oldest, id, pruneRows);
             break;
@@ -942,7 +941,7 @@
                 continue;
             }
 
-            if (oldest && oldest->mStart <= element->getSequence()) {
+            if (oldest && oldest->start() <= element->getSequence()) {
                 busy = true;
                 kickMe(oldest, id, pruneRows);
                 break;
@@ -953,7 +952,7 @@
         }
     }
 
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 
     return (pruneRows > 0) && busy;
 }
@@ -976,20 +975,20 @@
             // readers and let the clear run (below) deal with determining
             // if we are still blocked and return an error code to caller.
             if (busy) {
-                LogTimeEntry::wrlock();
+                LogReaderThread::wrlock();
                 LastLogTimes::iterator times = mTimes.begin();
                 while (times != mTimes.end()) {
-                    LogTimeEntry* entry = times->get();
+                    LogReaderThread* entry = times->get();
                     // Killer punch
-                    if (entry->isWatching(id)) {
+                    if (entry->IsWatching(id)) {
                         android::prdebug(
                                 "Kicking blocked reader, pid %d, from LogBuffer::clear()\n",
-                                entry->mClient->getPid());
+                                entry->client()->getPid());
                         entry->release_Locked();
                     }
                     times++;
                 }
-                LogTimeEntry::unlock();
+                LogReaderThread::unlock();
             }
         }
         wrlock();
@@ -1033,7 +1032,7 @@
 
 uint64_t LogBuffer::flushTo(SocketClient* reader, uint64_t start, pid_t* lastTid, bool privileged,
                             bool security,
-                            int (*filter)(const LogBufferElement* element, void* arg), void* arg) {
+                            const std::function<int(const LogBufferElement* element)>& filter) {
     LogBufferElementCollection::iterator it;
     uid_t uid = reader->getUid();
 
@@ -1071,7 +1070,7 @@
 
         // NB: calling out to another object with wrlock() held (safe)
         if (filter) {
-            int ret = (*filter)(element, arg);
+            int ret = filter(element);
             if (ret == false) {
                 continue;
             }
diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h
index 9a36712..eb41efb 100644
--- a/logd/LogBuffer.h
+++ b/logd/LogBuffer.h
@@ -27,9 +27,9 @@
 #include <sysutils/SocketClient.h>
 
 #include "LogBufferElement.h"
+#include "LogReaderThread.h"
 #include "LogStatistics.h"
 #include "LogTags.h"
-#include "LogTimes.h"
 #include "LogWhiteBlackList.h"
 
 //
@@ -115,8 +115,7 @@
     uint64_t flushTo(SocketClient* writer, uint64_t start,
                      pid_t* lastTid,  // &lastTid[LOG_ID_MAX] or nullptr
                      bool privileged, bool security,
-                     int (*filter)(const LogBufferElement* element, void* arg) = nullptr,
-                     void* arg = nullptr);
+                     const std::function<int(const LogBufferElement* element)>& filter);
 
     bool clear(log_id_t id, uid_t uid = AID_ROOT);
     unsigned long getSize(log_id_t id);
@@ -152,7 +151,7 @@
     static constexpr size_t maxPrune = 256;
 
     void maybePrune(log_id_t id);
-    void kickMe(LogTimeEntry* me, log_id_t id, unsigned long pruneRows);
+    void kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows);
 
     bool prune(log_id_t id, unsigned long pruneRows, uid_t uid = AID_ROOT);
     LogBufferElementCollection::iterator erase(
diff --git a/logd/LogKlog.cpp b/logd/LogKlog.cpp
index edd326a..5788ada 100644
--- a/logd/LogKlog.cpp
+++ b/logd/LogKlog.cpp
@@ -819,7 +819,7 @@
 
     // notify readers
     if (rc > 0) {
-        reader->notifyNewLog(static_cast<log_mask_t>(1 << LOG_ID_KERNEL));
+        reader->notifyNewLog(static_cast<unsigned int>(1 << LOG_ID_KERNEL));
     }
 
     return rc;
diff --git a/logd/LogListener.cpp b/logd/LogListener.cpp
index ba61042..138ab28 100644
--- a/logd/LogListener.cpp
+++ b/logd/LogListener.cpp
@@ -108,7 +108,7 @@
     int res = logbuf->log(logId, header->realtime, cred->uid, cred->pid, header->tid, msg,
                           ((size_t)n <= UINT16_MAX) ? (uint16_t)n : UINT16_MAX);
     if (res > 0) {
-        reader->notifyNewLog(static_cast<log_mask_t>(1 << logId));
+        reader->notifyNewLog(static_cast<unsigned int>(1 << logId));
     }
 
     return true;
diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp
index c6dea69..4702de5 100644
--- a/logd/LogReader.cpp
+++ b/logd/LogReader.cpp
@@ -39,20 +39,20 @@
 
 // When we are notified a new log entry is available, inform
 // listening sockets who are watching this entry's log id.
-void LogReader::notifyNewLog(log_mask_t log_mask) {
+void LogReader::notifyNewLog(unsigned int log_mask) {
     LastLogTimes& times = mLogbuf.mTimes;
 
-    LogTimeEntry::wrlock();
+    LogReaderThread::wrlock();
     for (const auto& entry : times) {
-        if (!entry->isWatchingMultiple(log_mask)) {
+        if (!entry->IsWatchingMultiple(log_mask)) {
             continue;
         }
-        if (entry->mTimeout.tv_sec || entry->mTimeout.tv_nsec) {
+        if (entry->timeout().tv_sec || entry->timeout().tv_nsec) {
             continue;
         }
         entry->triggerReader_Locked();
     }
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 }
 
 // Note returning false will release the SocketClient instance.
@@ -74,15 +74,15 @@
 
     // Clients are only allowed to send one command, disconnect them if they
     // send another.
-    LogTimeEntry::wrlock();
+    LogReaderThread::wrlock();
     for (const auto& entry : mLogbuf.mTimes) {
-        if (entry->mClient == cli) {
+        if (entry->client() == cli) {
             entry->release_Locked();
-            LogTimeEntry::unlock();
+            LogReaderThread::unlock();
             return false;
         }
     }
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 
     unsigned long tail = 0;
     static const char _tail[] = " tail=";
@@ -137,8 +137,8 @@
     if (!fastcmp<strncmp>(buffer, "dumpAndClose", 12)) {
         // Allow writer to get some cycles, and wait for pending notifications
         sched_yield();
-        LogTimeEntry::wrlock();
-        LogTimeEntry::unlock();
+        LogReaderThread::wrlock();
+        LogReaderThread::unlock();
         sched_yield();
         nonBlock = true;
     }
@@ -149,56 +149,37 @@
     uint64_t sequence = 1;
     // Convert realtime to sequence number
     if (start != log_time::EPOCH) {
-        class LogFindStart {
-            const pid_t mPid;
-            const unsigned mLogMask;
-            bool startTimeSet;
-            const log_time start;
-            uint64_t& sequence;
-            uint64_t last;
-            bool isMonotonic;
-
-          public:
-            LogFindStart(unsigned logMask, pid_t pid, log_time start, uint64_t& sequence,
-                         bool isMonotonic)
-                : mPid(pid),
-                  mLogMask(logMask),
-                  startTimeSet(false),
-                  start(start),
-                  sequence(sequence),
-                  last(sequence),
-                  isMonotonic(isMonotonic) {}
-
-            static int callback(const LogBufferElement* element, void* obj) {
-                LogFindStart* me = reinterpret_cast<LogFindStart*>(obj);
-                if ((!me->mPid || (me->mPid == element->getPid())) &&
-                    (me->mLogMask & (1 << element->getLogId()))) {
-                    if (me->start == element->getRealTime()) {
-                        me->sequence = element->getSequence();
-                        me->startTimeSet = true;
-                        return -1;
-                    } else if (!me->isMonotonic || android::isMonotonic(element->getRealTime())) {
-                        if (me->start < element->getRealTime()) {
-                            me->sequence = me->last;
-                            me->startTimeSet = true;
-                            return -1;
-                        }
-                        me->last = element->getSequence();
-                    } else {
-                        me->last = element->getSequence();
-                    }
-                }
-                return false;
+        bool start_time_set = false;
+        bool is_monotonic = logbuf().isMonotonic() && android::isMonotonic(start);
+        uint64_t last = sequence;
+        auto log_find_start = [pid, logMask, start, is_monotonic, &sequence, &start_time_set,
+                               &last](const LogBufferElement* element) -> int {
+            if (pid && pid != element->getPid()) {
+                return 0;
             }
+            if ((logMask & (1 << element->getLogId())) == 0) {
+                return 0;
+            }
+            if (start == element->getRealTime()) {
+                sequence = element->getSequence();
+                start_time_set = true;
+                return -1;
+            } else if (!is_monotonic || android::isMonotonic(element->getRealTime())) {
+                if (start < element->getRealTime()) {
+                    sequence = last;
+                    start_time_set = true;
+                    return -1;
+                }
+                last = element->getSequence();
+            } else {
+                last = element->getSequence();
+            }
+            return 0;
+        };
 
-            bool found() { return startTimeSet; }
-        } logFindStart(logMask, pid, start, sequence,
-                       logbuf().isMonotonic() && android::isMonotonic(start));
+        logbuf().flushTo(cli, sequence, nullptr, privileged, can_read_security, log_find_start);
 
-        logbuf().flushTo(cli, sequence, nullptr, privileged, can_read_security,
-                         logFindStart.callback, &logFindStart);
-
-        if (!logFindStart.found()) {
+        if (!start_time_set) {
             if (nonBlock) {
                 doSocketDelete(cli);
                 return false;
@@ -217,11 +198,12 @@
         timeout = 0;
     }
 
-    LogTimeEntry::wrlock();
-    auto entry = std::make_unique<LogTimeEntry>(*this, cli, nonBlock, tail, logMask, pid, start,
-                                                sequence, timeout, privileged, can_read_security);
+    LogReaderThread::wrlock();
+    auto entry =
+            std::make_unique<LogReaderThread>(*this, cli, nonBlock, tail, logMask, pid, start,
+                                              sequence, timeout, privileged, can_read_security);
     if (!entry->startReader_Locked()) {
-        LogTimeEntry::unlock();
+        LogReaderThread::unlock();
         return false;
     }
 
@@ -234,24 +216,24 @@
     setsockopt(cli->getSocket(), SOL_SOCKET, SO_SNDTIMEO, (const char*)&t,
                sizeof(t));
 
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 
     return true;
 }
 
 void LogReader::doSocketDelete(SocketClient* cli) {
     LastLogTimes& times = mLogbuf.mTimes;
-    LogTimeEntry::wrlock();
+    LogReaderThread::wrlock();
     LastLogTimes::iterator it = times.begin();
     while (it != times.end()) {
-        LogTimeEntry* entry = it->get();
-        if (entry->mClient == cli) {
+        LogReaderThread* entry = it->get();
+        if (entry->client() == cli) {
             entry->release_Locked();
             break;
         }
         it++;
     }
-    LogTimeEntry::unlock();
+    LogReaderThread::unlock();
 }
 
 int LogReader::getLogSocket() {
diff --git a/logd/LogReader.h b/logd/LogReader.h
index b5312b6..f00cc21 100644
--- a/logd/LogReader.h
+++ b/logd/LogReader.h
@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 
-#ifndef _LOGD_LOG_WRITER_H__
-#define _LOGD_LOG_WRITER_H__
+#pragma once
 
 #include <sysutils/SocketListener.h>
 
-#include "LogTimes.h"
+#include "LogReaderThread.h"
 
 #define LOGD_SNDTIMEO 32
 
@@ -30,7 +29,7 @@
 
    public:
     explicit LogReader(LogBuffer* logbuf);
-    void notifyNewLog(log_mask_t logMask);
+    void notifyNewLog(unsigned int logMask);
 
     LogBuffer& logbuf(void) const {
         return mLogbuf;
@@ -44,5 +43,3 @@
 
     void doSocketDelete(SocketClient* cli);
 };
-
-#endif
diff --git a/logd/LogReaderThread.cpp b/logd/LogReaderThread.cpp
new file mode 100644
index 0000000..5413c4d
--- /dev/null
+++ b/logd/LogReaderThread.cpp
@@ -0,0 +1,240 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "LogReaderThread.h"
+
+#include <errno.h>
+#include <string.h>
+#include <sys/prctl.h>
+
+#include <thread>
+
+#include "LogBuffer.h"
+#include "LogReader.h"
+
+using namespace std::placeholders;
+
+pthread_mutex_t LogReaderThread::timesLock = PTHREAD_MUTEX_INITIALIZER;
+
+LogReaderThread::LogReaderThread(LogReader& reader, SocketClient* client, bool non_block,
+                                 unsigned long tail, unsigned int log_mask, pid_t pid,
+                                 log_time start_time, uint64_t start, uint64_t timeout,
+                                 bool privileged, bool can_read_security_logs)
+    : leading_dropped_(false),
+      reader_(reader),
+      log_mask_(log_mask),
+      pid_(pid),
+      tail_(tail),
+      count_(0),
+      index_(0),
+      client_(client),
+      start_time_(start_time),
+      start_(start),
+      non_block_(non_block),
+      privileged_(privileged),
+      can_read_security_logs_(can_read_security_logs) {
+    timeout_.tv_sec = timeout / NS_PER_SEC;
+    timeout_.tv_nsec = timeout % NS_PER_SEC;
+    memset(last_tid_, 0, sizeof(last_tid_));
+    pthread_cond_init(&thread_triggered_condition_, nullptr);
+    cleanSkip_Locked();
+}
+
+bool LogReaderThread::startReader_Locked() {
+    auto thread = std::thread{&LogReaderThread::ThreadFunction, this};
+    thread.detach();
+    return true;
+}
+
+void LogReaderThread::ThreadFunction() {
+    prctl(PR_SET_NAME, "logd.reader.per");
+
+    SocketClient* client = client_;
+
+    LogBuffer& logbuf = reader_.logbuf();
+
+    leading_dropped_ = true;
+
+    wrlock();
+
+    uint64_t start = start_;
+
+    while (!release_) {
+        if (timeout_.tv_sec || timeout_.tv_nsec) {
+            if (pthread_cond_clockwait(&thread_triggered_condition_, &timesLock, CLOCK_MONOTONIC,
+                                       &timeout_) == ETIMEDOUT) {
+                timeout_.tv_sec = 0;
+                timeout_.tv_nsec = 0;
+            }
+            if (release_) {
+                break;
+            }
+        }
+
+        unlock();
+
+        if (tail_) {
+            logbuf.flushTo(client, start, nullptr, privileged_, can_read_security_logs_,
+                           std::bind(&LogReaderThread::FilterFirstPass, this, _1));
+            leading_dropped_ =
+                    true;  // TODO: Likely a bug, if leading_dropped_ was not true before calling
+                           // flushTo(), then it should not be reset to true after.
+        }
+        start = logbuf.flushTo(client, start, last_tid_, privileged_, can_read_security_logs_,
+                               std::bind(&LogReaderThread::FilterSecondPass, this, _1));
+
+        // We only ignore entries before the original start time for the first flushTo(), if we
+        // get entries after this first flush before the original start time, then the client
+        // wouldn't have seen them.
+        // Note: this is still racy and may skip out of order events that came in since the last
+        // time the client disconnected and then reconnected with the new start time.  The long term
+        // solution here is that clients must request events since a specific sequence number.
+        start_time_.tv_sec = 0;
+        start_time_.tv_nsec = 0;
+
+        wrlock();
+
+        if (start == LogBufferElement::FLUSH_ERROR) {
+            break;
+        }
+
+        start_ = start + 1;
+
+        if (non_block_ || release_) {
+            break;
+        }
+
+        cleanSkip_Locked();
+
+        if (!timeout_.tv_sec && !timeout_.tv_nsec) {
+            pthread_cond_wait(&thread_triggered_condition_, &timesLock);
+        }
+    }
+
+    LogReader& reader = reader_;
+    reader.release(client);
+
+    client->decRef();
+
+    LastLogTimes& times = reader.logbuf().mTimes;
+    auto it = std::find_if(times.begin(), times.end(),
+                           [this](const auto& other) { return other.get() == this; });
+
+    if (it != times.end()) {
+        times.erase(it);
+    }
+
+    unlock();
+}
+
+// A first pass to count the number of elements
+int LogReaderThread::FilterFirstPass(const LogBufferElement* element) {
+    LogReaderThread::wrlock();
+
+    if (leading_dropped_) {
+        if (element->getDropped()) {
+            LogReaderThread::unlock();
+            return false;
+        }
+        leading_dropped_ = false;
+    }
+
+    if (count_ == 0) {
+        start_ = element->getSequence();
+    }
+
+    if ((!pid_ || pid_ == element->getPid()) && IsWatching(element->getLogId()) &&
+        (start_time_ == log_time::EPOCH || start_time_ <= element->getRealTime())) {
+        ++count_;
+    }
+
+    LogReaderThread::unlock();
+
+    return false;
+}
+
+// A second pass to send the selected elements
+int LogReaderThread::FilterSecondPass(const LogBufferElement* element) {
+    LogReaderThread::wrlock();
+
+    start_ = element->getSequence();
+
+    if (skip_ahead_[element->getLogId()]) {
+        skip_ahead_[element->getLogId()]--;
+        goto skip;
+    }
+
+    if (leading_dropped_) {
+        if (element->getDropped()) {
+            goto skip;
+        }
+        leading_dropped_ = false;
+    }
+
+    // Truncate to close race between first and second pass
+    if (non_block_ && tail_ && index_ >= count_) {
+        goto stop;
+    }
+
+    if (!IsWatching(element->getLogId())) {
+        goto skip;
+    }
+
+    if (pid_ && pid_ != element->getPid()) {
+        goto skip;
+    }
+
+    if (start_time_ != log_time::EPOCH && element->getRealTime() <= start_time_) {
+        goto skip;
+    }
+
+    if (release_) {
+        goto stop;
+    }
+
+    if (!tail_) {
+        goto ok;
+    }
+
+    ++index_;
+
+    if (count_ > tail_ && index_ <= (count_ - tail_)) {
+        goto skip;
+    }
+
+    if (!non_block_) {
+        tail_ = 0;
+    }
+
+ok:
+    if (!skip_ahead_[element->getLogId()]) {
+        LogReaderThread::unlock();
+        return true;
+    }
+    // FALLTHRU
+
+skip:
+    LogReaderThread::unlock();
+    return false;
+
+stop:
+    LogReaderThread::unlock();
+    return -1;
+}
+
+void LogReaderThread::cleanSkip_Locked(void) {
+    memset(skip_ahead_, 0, sizeof(skip_ahead_));
+}
diff --git a/logd/LogReaderThread.h b/logd/LogReaderThread.h
new file mode 100644
index 0000000..39a8b63
--- /dev/null
+++ b/logd/LogReaderThread.h
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2012-2013 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <pthread.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <time.h>
+
+#include <list>
+#include <memory>
+
+#include <log/log.h>
+#include <sysutils/SocketClient.h>
+
+class LogReader;
+class LogBufferElement;
+
+class LogReaderThread {
+    static pthread_mutex_t timesLock;
+
+  public:
+    LogReaderThread(LogReader& reader, SocketClient* client, bool non_block, unsigned long tail,
+                    unsigned int log_mask, pid_t pid, log_time start_time, uint64_t sequence,
+                    uint64_t timeout, bool privileged, bool can_read_security_logs);
+
+    // Protect List manipulations
+    static void wrlock() { pthread_mutex_lock(&timesLock); }
+    static void rdlock() { pthread_mutex_lock(&timesLock); }
+    static void unlock() { pthread_mutex_unlock(&timesLock); }
+
+    bool startReader_Locked();
+
+    void triggerReader_Locked() { pthread_cond_signal(&thread_triggered_condition_); }
+
+    void triggerSkip_Locked(log_id_t id, unsigned int skip) { skip_ahead_[id] = skip; }
+    void cleanSkip_Locked();
+
+    void release_Locked() {
+        // gracefully shut down the socket.
+        shutdown(client_->getSocket(), SHUT_RDWR);
+        release_ = true;
+        pthread_cond_signal(&thread_triggered_condition_);
+    }
+
+    bool IsWatching(log_id_t id) const { return log_mask_ & (1 << id); }
+    bool IsWatchingMultiple(unsigned int log_mask) const { return log_mask_ & log_mask; }
+
+    const SocketClient* client() const { return client_; }
+    uint64_t start() const { return start_; }
+    const timespec& timeout() const { return timeout_; }
+
+  private:
+    void ThreadFunction();
+    // flushTo filter callbacks
+    int FilterFirstPass(const LogBufferElement* element);
+    int FilterSecondPass(const LogBufferElement* element);
+
+    // Set to true to cause the thread to end and the LogReaderThread to delete itself.
+    bool release_ = false;
+    // Indicates whether or not 'leading' (first logs seen starting from start_) 'dropped' (chatty)
+    // messages should be ignored.
+    bool leading_dropped_;
+
+    // Condition variable for waking the reader thread if there are messages pending for its client.
+    pthread_cond_t thread_triggered_condition_;
+
+    // Reference to the parent thread that manages log reader sockets.
+    LogReader& reader_;
+    // A mask of the logs buffers that are read by this reader.
+    const unsigned int log_mask_;
+    // If set to non-zero, only pids equal to this are read by the reader.
+    const pid_t pid_;
+    // When a reader is referencing (via start_) old elements in the log buffer, and the log
+    // buffer's size grows past its memory limit, the log buffer may request the reader to skip
+    // ahead a specified number of logs.
+    unsigned int skip_ahead_[LOG_ID_MAX];
+    // Used for distinguishing 'dropped' messages for duplicate logs vs chatty drops
+    pid_t last_tid_[LOG_ID_MAX];
+
+    // These next three variables are used for reading only the most recent lines aka `adb logcat
+    // -t` / `adb logcat -T`.
+    // tail_ is the number of most recent lines to print.
+    unsigned long tail_;
+    // count_ is the result of a first pass through the log buffer to determine how many total
+    // messages there are.
+    unsigned long count_;
+    // index_ is used along with count_ to only start sending lines once index_ > (count_ - tail_)
+    // and to disconnect the reader (if it is dumpAndClose, `adb logcat -t`), when index_ >= count_.
+    unsigned long index_;
+
+    // A pointer to the socket for this reader.
+    SocketClient* client_;
+    // When a reader requests logs starting from a given timestamp, its stored here for the first
+    // pass, such that logs before this time stamp that are accumulated in the buffer are ignored.
+    log_time start_time_;
+    // The point from which the reader will read logs once awoken.
+    uint64_t start_;
+    // CLOCK_MONOTONIC based timeout used for log wrapping.  If this timeout expires before logs
+    // wrap, then wake up and send the logs to the reader anyway.
+    timespec timeout_;
+    // If this reader is 'dumpAndClose' and will disconnect once it has read its intended logs.
+    const bool non_block_;
+
+    // Whether or not this reader can read logs from all UIDs or only its own UID.  See
+    // clientHasLogCredentials().
+    bool privileged_;
+    // Whether or not this reader can read security logs.  See CanReadSecurityLogs().
+    bool can_read_security_logs_;
+};
+
+typedef std::list<std::unique_ptr<LogReaderThread>> LastLogTimes;
diff --git a/logd/LogTimes.cpp b/logd/LogTimes.cpp
deleted file mode 100644
index ad150bd..0000000
--- a/logd/LogTimes.cpp
+++ /dev/null
@@ -1,253 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <errno.h>
-#include <string.h>
-#include <sys/prctl.h>
-
-#include "LogBuffer.h"
-#include "LogReader.h"
-#include "LogTimes.h"
-
-pthread_mutex_t LogTimeEntry::timesLock = PTHREAD_MUTEX_INITIALIZER;
-
-LogTimeEntry::LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBlock,
-                           unsigned long tail, log_mask_t logMask, pid_t pid, log_time start_time,
-                           uint64_t start, uint64_t timeout, bool privileged,
-                           bool can_read_security_logs)
-    : leadingDropped(false),
-      mReader(reader),
-      mLogMask(logMask),
-      mPid(pid),
-      mCount(0),
-      mTail(tail),
-      mIndex(0),
-      mClient(client),
-      mStartTime(start_time),
-      mStart(start),
-      mNonBlock(nonBlock),
-      privileged_(privileged),
-      can_read_security_logs_(can_read_security_logs) {
-    mTimeout.tv_sec = timeout / NS_PER_SEC;
-    mTimeout.tv_nsec = timeout % NS_PER_SEC;
-    memset(mLastTid, 0, sizeof(mLastTid));
-    pthread_cond_init(&threadTriggeredCondition, nullptr);
-    cleanSkip_Locked();
-}
-
-bool LogTimeEntry::startReader_Locked() {
-    pthread_attr_t attr;
-
-    if (!pthread_attr_init(&attr)) {
-        if (!pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED)) {
-            if (!pthread_create(&mThread, &attr, LogTimeEntry::threadStart,
-                                this)) {
-                pthread_attr_destroy(&attr);
-                return true;
-            }
-        }
-        pthread_attr_destroy(&attr);
-    }
-
-    return false;
-}
-
-void* LogTimeEntry::threadStart(void* obj) {
-    prctl(PR_SET_NAME, "logd.reader.per");
-
-    LogTimeEntry* me = reinterpret_cast<LogTimeEntry*>(obj);
-
-    SocketClient* client = me->mClient;
-
-    LogBuffer& logbuf = me->mReader.logbuf();
-
-    me->leadingDropped = true;
-
-    wrlock();
-
-    uint64_t start = me->mStart;
-
-    while (!me->mRelease) {
-        if (me->mTimeout.tv_sec || me->mTimeout.tv_nsec) {
-            if (pthread_cond_clockwait(&me->threadTriggeredCondition, &timesLock, CLOCK_MONOTONIC,
-                                       &me->mTimeout) == ETIMEDOUT) {
-                me->mTimeout.tv_sec = 0;
-                me->mTimeout.tv_nsec = 0;
-            }
-            if (me->mRelease) {
-                break;
-            }
-        }
-
-        unlock();
-
-        if (me->mTail) {
-            logbuf.flushTo(client, start, nullptr, me->privileged_, me->can_read_security_logs_,
-                           FilterFirstPass, me);
-            me->leadingDropped = true;
-        }
-        start = logbuf.flushTo(client, start, me->mLastTid, me->privileged_,
-                               me->can_read_security_logs_, FilterSecondPass, me);
-
-        // We only ignore entries before the original start time for the first flushTo(), if we
-        // get entries after this first flush before the original start time, then the client
-        // wouldn't have seen them.
-        // Note: this is still racy and may skip out of order events that came in since the last
-        // time the client disconnected and then reconnected with the new start time.  The long term
-        // solution here is that clients must request events since a specific sequence number.
-        me->mStartTime.tv_sec = 0;
-        me->mStartTime.tv_nsec = 0;
-
-        wrlock();
-
-        if (start == LogBufferElement::FLUSH_ERROR) {
-            break;
-        }
-
-        me->mStart = start + 1;
-
-        if (me->mNonBlock || me->mRelease) {
-            break;
-        }
-
-        me->cleanSkip_Locked();
-
-        if (!me->mTimeout.tv_sec && !me->mTimeout.tv_nsec) {
-            pthread_cond_wait(&me->threadTriggeredCondition, &timesLock);
-        }
-    }
-
-    LogReader& reader = me->mReader;
-    reader.release(client);
-
-    client->decRef();
-
-    LastLogTimes& times = reader.logbuf().mTimes;
-    auto it =
-        std::find_if(times.begin(), times.end(),
-                     [&me](const auto& other) { return other.get() == me; });
-
-    if (it != times.end()) {
-        times.erase(it);
-    }
-
-    unlock();
-
-    return nullptr;
-}
-
-// A first pass to count the number of elements
-int LogTimeEntry::FilterFirstPass(const LogBufferElement* element, void* obj) {
-    LogTimeEntry* me = reinterpret_cast<LogTimeEntry*>(obj);
-
-    LogTimeEntry::wrlock();
-
-    if (me->leadingDropped) {
-        if (element->getDropped()) {
-            LogTimeEntry::unlock();
-            return false;
-        }
-        me->leadingDropped = false;
-    }
-
-    if (me->mCount == 0) {
-        me->mStart = element->getSequence();
-    }
-
-    if ((!me->mPid || me->mPid == element->getPid()) && me->isWatching(element->getLogId()) &&
-        (me->mStartTime == log_time::EPOCH || me->mStartTime <= element->getRealTime())) {
-        ++me->mCount;
-    }
-
-    LogTimeEntry::unlock();
-
-    return false;
-}
-
-// A second pass to send the selected elements
-int LogTimeEntry::FilterSecondPass(const LogBufferElement* element, void* obj) {
-    LogTimeEntry* me = reinterpret_cast<LogTimeEntry*>(obj);
-
-    LogTimeEntry::wrlock();
-
-    me->mStart = element->getSequence();
-
-    if (me->skipAhead[element->getLogId()]) {
-        me->skipAhead[element->getLogId()]--;
-        goto skip;
-    }
-
-    if (me->leadingDropped) {
-        if (element->getDropped()) {
-            goto skip;
-        }
-        me->leadingDropped = false;
-    }
-
-    // Truncate to close race between first and second pass
-    if (me->mNonBlock && me->mTail && (me->mIndex >= me->mCount)) {
-        goto stop;
-    }
-
-    if (!me->isWatching(element->getLogId())) {
-        goto skip;
-    }
-
-    if (me->mPid && (me->mPid != element->getPid())) {
-        goto skip;
-    }
-
-    if (me->mStartTime != log_time::EPOCH && element->getRealTime() <= me->mStartTime) {
-        goto skip;
-    }
-
-    if (me->mRelease) {
-        goto stop;
-    }
-
-    if (!me->mTail) {
-        goto ok;
-    }
-
-    ++me->mIndex;
-
-    if ((me->mCount > me->mTail) && (me->mIndex <= (me->mCount - me->mTail))) {
-        goto skip;
-    }
-
-    if (!me->mNonBlock) {
-        me->mTail = 0;
-    }
-
-ok:
-    if (!me->skipAhead[element->getLogId()]) {
-        LogTimeEntry::unlock();
-        return true;
-    }
-// FALLTHRU
-
-skip:
-    LogTimeEntry::unlock();
-    return false;
-
-stop:
-    LogTimeEntry::unlock();
-    return -1;
-}
-
-void LogTimeEntry::cleanSkip_Locked(void) {
-    memset(skipAhead, 0, sizeof(skipAhead));
-}
diff --git a/logd/LogTimes.h b/logd/LogTimes.h
deleted file mode 100644
index 56c930a..0000000
--- a/logd/LogTimes.h
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- * Copyright (C) 2012-2013 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef _LOGD_LOG_TIMES_H__
-#define _LOGD_LOG_TIMES_H__
-
-#include <pthread.h>
-#include <sys/socket.h>
-#include <sys/types.h>
-#include <time.h>
-
-#include <list>
-#include <memory>
-
-#include <log/log.h>
-#include <sysutils/SocketClient.h>
-
-typedef unsigned int log_mask_t;
-
-class LogReader;
-class LogBufferElement;
-
-class LogTimeEntry {
-    static pthread_mutex_t timesLock;
-    bool mRelease = false;
-    bool leadingDropped;
-    pthread_cond_t threadTriggeredCondition;
-    pthread_t mThread;
-    LogReader& mReader;
-    static void* threadStart(void* me);
-    const log_mask_t mLogMask;
-    const pid_t mPid;
-    unsigned int skipAhead[LOG_ID_MAX];
-    pid_t mLastTid[LOG_ID_MAX];
-    unsigned long mCount;
-    unsigned long mTail;
-    unsigned long mIndex;
-
-  public:
-    LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBlock, unsigned long tail,
-                 log_mask_t logMask, pid_t pid, log_time start_time, uint64_t sequence,
-                 uint64_t timeout, bool privileged, bool can_read_security_logs);
-
-    SocketClient* mClient;
-    log_time mStartTime;
-    uint64_t mStart;
-    struct timespec mTimeout;  // CLOCK_MONOTONIC based timeout used for log wrapping.
-    const bool mNonBlock;
-
-    // Protect List manipulations
-    static void wrlock(void) {
-        pthread_mutex_lock(&timesLock);
-    }
-    static void rdlock(void) {
-        pthread_mutex_lock(&timesLock);
-    }
-    static void unlock(void) {
-        pthread_mutex_unlock(&timesLock);
-    }
-
-    bool startReader_Locked();
-
-    void triggerReader_Locked(void) {
-        pthread_cond_signal(&threadTriggeredCondition);
-    }
-
-    void triggerSkip_Locked(log_id_t id, unsigned int skip) {
-        skipAhead[id] = skip;
-    }
-    void cleanSkip_Locked(void);
-
-    void release_Locked(void) {
-        // gracefully shut down the socket.
-        shutdown(mClient->getSocket(), SHUT_RDWR);
-        mRelease = true;
-        pthread_cond_signal(&threadTriggeredCondition);
-    }
-
-    bool isWatching(log_id_t id) const {
-        return mLogMask & (1 << id);
-    }
-    bool isWatchingMultiple(log_mask_t logMask) const {
-        return mLogMask & logMask;
-    }
-    // flushTo filter callbacks
-    static int FilterFirstPass(const LogBufferElement* element, void* me);
-    static int FilterSecondPass(const LogBufferElement* element, void* me);
-
-  private:
-    bool privileged_;
-    bool can_read_security_logs_;
-};
-
-typedef std::list<std::unique_ptr<LogTimeEntry>> LastLogTimes;
-
-#endif  // _LOGD_LOG_TIMES_H__
diff --git a/logd/fuzz/log_buffer_log_fuzzer.cpp b/logd/fuzz/log_buffer_log_fuzzer.cpp
index 14c5163..8156612 100644
--- a/logd/fuzz/log_buffer_log_fuzzer.cpp
+++ b/logd/fuzz/log_buffer_log_fuzzer.cpp
@@ -16,7 +16,7 @@
 #include <string>
 
 #include "../LogBuffer.h"
-#include "../LogTimes.h"
+#include "../LogReaderThread.h"
 
 // We don't want to waste a lot of entropy on messages
 #define MAX_MSG_LENGTH 5