From 4c0078d67a552c63b439b2398e17e9cdaf6b7067 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 28 Jun 2018 18:43:19 -0700 Subject: [PATCH] adbd: fix spurious failure to create dirs when pushing. When pushing to a path, we first try to ensure the directory path exists and has the permissions expected by fs_config. Due to a change that changed the fs_config check from a blacklist to a whitelist, we started doing this for /data (which doesn't begin with /data/), and the UID/GID for that path was accidentally being reused for following path segments that didn't exist, leading to a failed attempt to chown /data/local/tmp/foo to be owned by system. Bug: http://b/110953234 Test: python test_device.py Change-Id: Ie798eec48bcf54aea40f6d90cc03bb2170280ee8 --- adb/daemon/file_sync_service.cpp | 18 ++++++++++++------ adb/test_device.py | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/adb/daemon/file_sync_service.cpp b/adb/daemon/file_sync_service.cpp index 11289934e..f7be8f94b 100644 --- a/adb/daemon/file_sync_service.cpp +++ b/adb/daemon/file_sync_service.cpp @@ -69,17 +69,23 @@ static bool update_capabilities(const char* path, uint64_t capabilities) { } static bool secure_mkdirs(const std::string& path) { - uid_t uid = -1; - gid_t gid = -1; - unsigned int mode = 0775; - uint64_t capabilities = 0; - if (path[0] != '/') return false; std::vector path_components = android::base::Split(path, "/"); std::string partial_path; for (const auto& path_component : path_components) { - if (partial_path.back() != OS_PATH_SEPARATOR) partial_path += OS_PATH_SEPARATOR; + uid_t uid = -1; + gid_t gid = -1; + unsigned int mode = 0775; + uint64_t capabilities = 0; + + if (path_component.empty()) { + continue; + } + + if (partial_path.empty() || partial_path.back() != OS_PATH_SEPARATOR) { + partial_path += OS_PATH_SEPARATOR; + } partial_path += path_component; if (should_use_fs_config(partial_path)) { diff --git a/adb/test_device.py b/adb/test_device.py index f995be288..5aa268411 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -866,6 +866,21 @@ class FileOperationsTest(DeviceTest): self.assertTrue('Permission denied' in output or 'Read-only file system' in output) + @requires_non_root + def test_push_directory_creation(self): + """Regression test for directory creation. + + Bug: http://b/110953234 + """ + with tempfile.NamedTemporaryFile() as tmp_file: + tmp_file.write('\0' * 1024 * 1024) + tmp_file.flush() + remote_path = self.DEVICE_TEMP_DIR + '/test_push_directory_creation' + self.device.shell(['rm', '-rf', remote_path]) + + remote_path += '/filename' + self.device.push(local=tmp_file.name, remote=remote_path) + def _test_pull(self, remote_file, checksum): tmp_write = tempfile.NamedTemporaryFile(mode='wb', delete=False) tmp_write.close()