From 786dac3d5068594735ef38f4a51bbb89e9a12003 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Thu, 15 Jun 2023 16:23:56 -0700 Subject: [PATCH] Update some fs_mgr/debuggerd to use getpagesize() instead of PAGE_SIZE Test: th Bug: 279808236 Change-Id: I9d30cfe19d2b1a7d624cc5425e4315dc6e3b2ad2 --- debuggerd/crasher/crasher.cpp | 2 +- debuggerd/debuggerd_test.cpp | 19 +------------------ debuggerd/handler/debuggerd_handler.cpp | 8 ++++---- debuggerd/libdebuggerd/scudo.cpp | 11 ++++++----- fs_mgr/fs_mgr.cpp | 7 ++++--- fs_mgr/fs_mgr_format.cpp | 10 +++++++++- fs_mgr/fs_mgr_overlayfs.cpp | 4 +++- 7 files changed, 28 insertions(+), 33 deletions(-) diff --git a/debuggerd/crasher/crasher.cpp b/debuggerd/crasher/crasher.cpp index 6a1987882..12ba502ec 100644 --- a/debuggerd/crasher/crasher.cpp +++ b/debuggerd/crasher/crasher.cpp @@ -148,7 +148,7 @@ noinline void abuse_heap() { noinline void leak() { while (true) { void* mapping = - mmap(nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + mmap(nullptr, getpagesize(), PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); static_cast(mapping)[0] = 'a'; } } diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index 4cd619337..52c1c2554 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -300,24 +300,7 @@ void CrasherTest::AssertDeath(int signo) { } static void ConsumeFd(unique_fd fd, std::string* output) { - constexpr size_t read_length = PAGE_SIZE; - std::string result; - - while (true) { - size_t offset = result.size(); - result.resize(result.size() + PAGE_SIZE); - ssize_t rc = TEMP_FAILURE_RETRY(read(fd.get(), &result[offset], read_length)); - if (rc == -1) { - FAIL() << "read failed: " << strerror(errno); - } else if (rc == 0) { - result.resize(result.size() - PAGE_SIZE); - break; - } - - result.resize(result.size() - PAGE_SIZE + rc); - } - - *output = std::move(result); + ASSERT_TRUE(android::base::ReadFdToString(fd, output)); } class LogcatCollector { diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index c6a535ad6..1e5365d3c 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -721,19 +721,19 @@ void debuggerd_init(debuggerd_callbacks_t* callbacks) { } size_t thread_stack_pages = 8; - void* thread_stack_allocation = mmap(nullptr, PAGE_SIZE * (thread_stack_pages + 2), PROT_NONE, + void* thread_stack_allocation = mmap(nullptr, getpagesize() * (thread_stack_pages + 2), PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); if (thread_stack_allocation == MAP_FAILED) { fatal_errno("failed to allocate debuggerd thread stack"); } - char* stack = static_cast(thread_stack_allocation) + PAGE_SIZE; - if (mprotect(stack, PAGE_SIZE * thread_stack_pages, PROT_READ | PROT_WRITE) != 0) { + char* stack = static_cast(thread_stack_allocation) + getpagesize(); + if (mprotect(stack, getpagesize() * thread_stack_pages, PROT_READ | PROT_WRITE) != 0) { fatal_errno("failed to mprotect debuggerd thread stack"); } // Stack grows negatively, set it to the last byte in the page... - stack = (stack + thread_stack_pages * PAGE_SIZE - 1); + stack = (stack + thread_stack_pages * getpagesize() - 1); // and align it. stack -= 15; pseudothread_stack = stack; diff --git a/debuggerd/libdebuggerd/scudo.cpp b/debuggerd/libdebuggerd/scudo.cpp index 5a62fe1e7..837f40612 100644 --- a/debuggerd/libdebuggerd/scudo.cpp +++ b/debuggerd/libdebuggerd/scudo.cpp @@ -22,6 +22,7 @@ #include #include +#include #include "tombstone.pb.h" @@ -54,21 +55,21 @@ ScudoCrashData::ScudoCrashData(unwindstack::Memory* process_memory, } untagged_fault_addr_ = process_info.untagged_fault_address; - uintptr_t fault_page = untagged_fault_addr_ & ~(PAGE_SIZE - 1); + uintptr_t fault_page = untagged_fault_addr_ & ~(getpagesize() - 1); - uintptr_t memory_begin = fault_page - PAGE_SIZE * 16; + uintptr_t memory_begin = fault_page - getpagesize() * 16; if (memory_begin > fault_page) { return; } - uintptr_t memory_end = fault_page + PAGE_SIZE * 16; + uintptr_t memory_end = fault_page + getpagesize() * 16; if (memory_end < fault_page) { return; } auto memory = std::make_unique(memory_end - memory_begin); - for (auto i = memory_begin; i != memory_end; i += PAGE_SIZE) { - process_memory->ReadFully(i, memory.get() + i - memory_begin, PAGE_SIZE); + for (auto i = memory_begin; i != memory_end; i += getpagesize()) { + process_memory->ReadFully(i, memory.get() + i - memory_begin, getpagesize()); } auto memory_tags = std::make_unique((memory_end - memory_begin) / kTagGranuleSize); diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 7f4959f7f..e568a9bd6 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -613,7 +613,6 @@ static void tune_metadata_csum(const std::string& blk_device, const FstabEntry& // Read the primary superblock from an f2fs filesystem. On failure return // false. If it's not an f2fs filesystem, also set FS_STAT_INVALID_MAGIC. -#define F2FS_BLKSIZE 4096 #define F2FS_SUPER_OFFSET 1024 static bool read_f2fs_superblock(const std::string& blk_device, int* fs_stat) { android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(blk_device.c_str(), O_RDONLY | O_CLOEXEC))); @@ -628,7 +627,9 @@ static bool read_f2fs_superblock(const std::string& blk_device, int* fs_stat) { PERROR << "Can't read '" << blk_device << "' superblock1"; return false; } - if (TEMP_FAILURE_RETRY(pread(fd, &sb2, sizeof(sb2), F2FS_BLKSIZE + F2FS_SUPER_OFFSET)) != + // F2FS only supports block_size=page_size case. So, it is safe to call + // `getpagesize()` and use that as size of super block. + if (TEMP_FAILURE_RETRY(pread(fd, &sb2, sizeof(sb2), getpagesize() + F2FS_SUPER_OFFSET)) != sizeof(sb2)) { PERROR << "Can't read '" << blk_device << "' superblock2"; return false; @@ -652,7 +653,7 @@ bool fs_mgr_is_f2fs(const std::string& blk_device) { return false; } if (sb == cpu_to_le32(F2FS_SUPER_MAGIC)) return true; - if (TEMP_FAILURE_RETRY(pread(fd, &sb, sizeof(sb), F2FS_BLKSIZE + F2FS_SUPER_OFFSET)) != + if (TEMP_FAILURE_RETRY(pread(fd, &sb, sizeof(sb), getpagesize() + F2FS_SUPER_OFFSET)) != sizeof(sb)) { return false; } diff --git a/fs_mgr/fs_mgr_format.cpp b/fs_mgr/fs_mgr_format.cpp index 7385f7961..622f18135 100644 --- a/fs_mgr/fs_mgr_format.cpp +++ b/fs_mgr/fs_mgr_format.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include "fs_mgr_priv.h" @@ -68,6 +69,13 @@ static int format_ext4(const std::string& fs_blkdev, const std::string& fs_mnt_p /* Format the partition using the calculated length */ + // EXT4 supports 4K block size on 16K page sizes. A 4K block + // size formatted EXT4 partition will mount fine on both 4K and 16K page + // size kernels. + // However, EXT4 does not support 16K block size on 4K systems. + // If we want the same userspace code to work on both 4k/16k kernels, + // using a hardcoded 4096 block size is a simple solution. Using + // getpagesize() here would work as well, but 4096 is simpler. std::string size_str = std::to_string(dev_sz / 4096); std::vector mke2fs_args = {"/system/bin/mke2fs", "-t", "ext4", "-b", "4096"}; @@ -127,7 +135,7 @@ static int format_f2fs(const std::string& fs_blkdev, uint64_t dev_sz, bool needs /* Format the partition using the calculated length */ - std::string size_str = std::to_string(dev_sz / 4096); + const auto size_str = std::to_string(dev_sz / getpagesize()); std::vector args = {"/system/bin/make_f2fs", "-g", "android"}; if (needs_projid) { diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index f04fc8d2f..01827d6cc 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -939,7 +939,9 @@ bool MakeScratchFilesystem(const std::string& scratch_device) { auto command = ""s; if (!access(kMkF2fs.c_str(), X_OK) && fs_mgr_filesystem_available("f2fs")) { fs_type = "f2fs"; - command = kMkF2fs + " -w 4096 -f -d1 -l" + android::base::Basename(kScratchMountPoint); + command = kMkF2fs + " -w "; + command += std::to_string(getpagesize()); + command += " -f -d1 -l" + android::base::Basename(kScratchMountPoint); } else if (!access(kMkExt4.c_str(), X_OK) && fs_mgr_filesystem_available("ext4")) { fs_type = "ext4"; command = kMkExt4 + " -F -b 4096 -t ext4 -m 0 -O has_journal -M " + kScratchMountPoint;