Merge "libdm: Fix race conditions in LoopControl::Attach." (4605a960) · Commits · e / os / android_system_core

fs_mgr/libdm/Android.bp

+1 −0

Original line number	Diff line number	Diff line
		@@ -27,6 +27,7 @@ cc_library_static {
		"dm_target.cpp",
		"dm.cpp",
		"loop_control.cpp",
		"utility.cpp",
		],

		static_libs: [

fs_mgr/libdm/dm.cpp

+3 −24

Original line number	Diff line number	Diff line
		@@ -29,6 +29,8 @@
		#include <android-base/strings.h>
		#include <uuid/uuid.h>

		#include "utility.h"

		namespace android {
		namespace dm {

		@@ -94,20 +96,6 @@ bool DeviceMapper::DeleteDevice(const std::string& name) {
		return true;
		}

		bool WaitForCondition(const std::function<bool()>& condition,
		const std::chrono::milliseconds& timeout_ms) {
		auto start_time = std::chrono::steady_clock::now();
		while (true) {
		if (condition()) return true;

		std::this_thread::sleep_for(20ms);

		auto now = std::chrono::steady_clock::now();
		auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
		if (time_elapsed > timeout_ms) return false;
		}
		}

		static std::string GenerateUuid() {
		uuid_t uuid_bytes;
		uuid_generate(uuid_bytes);
		@@ -138,16 +126,7 @@ bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table, s
		if (timeout_ms <= std::chrono::milliseconds::zero()) {
		return true;
		}

		auto condition = [&]() -> bool {
		// If the file exists but returns EPERM or something, we consider the
		// condition met.
		if (access(unique_path.c_str(), F_OK) != 0) {
		if (errno == ENOENT) return false;
		}
		return true;
		};
		if (!WaitForCondition(condition, timeout_ms)) {
		if (!WaitForFile(unique_path, timeout_ms)) {
		LOG(ERROR) << "Timed out waiting for device path: " << unique_path;
		DeleteDevice(name);
		return false;

fs_mgr/libdm/dm_test.cpp

+4 −4

Original line number	Diff line number	Diff line
		@@ -103,9 +103,9 @@ TEST(libdm, DmLinear) {
		ASSERT_TRUE(android::base::WriteFully(tmp1, message1, sizeof(message1)));
		ASSERT_TRUE(android::base::WriteFully(tmp2, message2, sizeof(message2)));

		LoopDevice loop_a(tmp1);
		LoopDevice loop_a(tmp1, 10s);
		ASSERT_TRUE(loop_a.valid());
		LoopDevice loop_b(tmp2);
		LoopDevice loop_b(tmp2, 10s);
		ASSERT_TRUE(loop_b.valid());

		// Define a 2-sector device, with each sector mapping to the first sector
		@@ -255,9 +255,9 @@ void SnapshotTestHarness::SetupImpl() {
		cow_fd_ = CreateTempFile("cow_device", kCowDeviceSize);
		ASSERT_GE(cow_fd_, 0);

		base_loop_ = std::make_unique<LoopDevice>(base_fd_);
		base_loop_ = std::make_unique<LoopDevice>(base_fd_, 10s);
		ASSERT_TRUE(base_loop_->valid());
		cow_loop_ = std::make_unique<LoopDevice>(cow_fd_);
		cow_loop_ = std::make_unique<LoopDevice>(cow_fd_, 10s);
		ASSERT_TRUE(cow_loop_->valid());

		DmTable origin_table;

fs_mgr/libdm/include/libdm/loop_control.h

+15 −6

Original line number	Diff line number	Diff line
		@@ -17,6 +17,7 @@
		#ifndef _LIBDM_LOOP_CONTROL_H_
		#define _LIBDM_LOOP_CONTROL_H_

		#include <chrono>
		#include <string>

		#include <android-base/unique_fd.h>
		@@ -29,8 +30,15 @@ class LoopControl final {
		LoopControl();

		// Attaches the file specified by 'file_fd' to the loop device specified
		// by 'loopdev'
		bool Attach(int file_fd, std::string* loopdev) const;
		// by 'loopdev'. It is possible that in between allocating and attaching
		// a loop device, another process attaches to the chosen loop device. If
		// this happens, Attach() will retry for up to \|timeout_ms\|. The timeout
		// should not be zero.
		//
		// The caller does not have to call WaitForFile(); it is implicitly called.
		// The given \|timeout_ms\| covers both potential sources of timeout.
		bool Attach(int file_fd, const std::chrono::milliseconds& timeout_ms,
		std::string* loopdev) const;

		// Detach the loop device given by 'loopdev' from the attached backing file.
		bool Detach(const std::string& loopdev) const;
		@@ -56,13 +64,13 @@ class LoopDevice {
		public:
		// Create a loop device for the given file descriptor. It is closed when
		// LoopDevice is destroyed only if auto_close is true.
		LoopDevice(int fd, bool auto_close = false);
		LoopDevice(int fd, const std::chrono::milliseconds& timeout_ms, bool auto_close = false);
		// Create a loop device for the given file path. It will be opened for
		// reading and writing and closed when the loop device is detached.
		explicit LoopDevice(const std::string& path);
		LoopDevice(const std::string& path, const std::chrono::milliseconds& timeout_ms);
		~LoopDevice();

		bool valid() const { return fd_ != -1 && !device_.empty(); }
		bool valid() const { return valid_; }
		const std::string& device() const { return device_; }

		LoopDevice(const LoopDevice&) = delete;
		@@ -71,12 +79,13 @@ class LoopDevice {
		LoopDevice(LoopDevice&&) = default;

		private:
		void Init();
		void Init(const std::chrono::milliseconds& timeout_ms);

		android::base::unique_fd fd_;
		bool owns_fd_;
		std::string device_;
		LoopControl control_;
		bool valid_ = false;
		};

		} // namespace dm

fs_mgr/libdm/loop_control.cpp

+44 −21

Original line number	Diff line number	Diff line
		@@ -27,6 +27,8 @@
		#include <android-base/stringprintf.h>
		#include <android-base/unique_fd.h>

		#include "utility.h"

		namespace android {
		namespace dm {

		@@ -37,21 +39,40 @@ LoopControl::LoopControl() : control_fd_(-1) {
		}
		}

		bool LoopControl::Attach(int file_fd, std::string* loopdev) const {
		bool LoopControl::Attach(int file_fd, const std::chrono::milliseconds& timeout_ms,
		std::string* loopdev) const {
		auto start_time = std::chrono::steady_clock::now();
		auto condition = [&]() -> WaitResult {
		if (!FindFreeLoopDevice(loopdev)) {
		LOG(ERROR) << "Failed to attach, no free loop devices";
		return false;
		return WaitResult::Fail;
		}

		auto now = std::chrono::steady_clock::now();
		auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
		if (!WaitForFile(*loopdev, timeout_ms - time_elapsed)) {
		LOG(ERROR) << "Timed out waiting for path: " << *loopdev;
		return WaitResult::Fail;
		}

		android::base::unique_fd loop_fd(TEMP_FAILURE_RETRY(open(loopdev->c_str(), O_RDWR \| O_CLOEXEC)));
		android::base::unique_fd loop_fd(
		TEMP_FAILURE_RETRY(open(loopdev->c_str(), O_RDWR \| O_CLOEXEC)));
		if (loop_fd < 0) {
		PLOG(ERROR) << "Failed to open: " << *loopdev;
		return false;
		return WaitResult::Fail;
		}

		int rc = ioctl(loop_fd, LOOP_SET_FD, file_fd);
		if (rc < 0) {
		if (int rc = ioctl(loop_fd, LOOP_SET_FD, file_fd); rc == 0) {
		return WaitResult::Done;
		}
		if (errno != EBUSY) {
		PLOG(ERROR) << "Failed LOOP_SET_FD";
		return WaitResult::Fail;
		}
		return WaitResult::Wait;
		};
		if (!WaitForCondition(condition, timeout_ms)) {
		LOG(ERROR) << "Timed out trying to acquire a loop device";
		return false;
		}
		return true;
		@@ -112,17 +133,19 @@ bool LoopControl::EnableDirectIo(int fd) {
		return true;
		}

		LoopDevice::LoopDevice(int fd, bool auto_close) : fd_(fd), owns_fd_(auto_close) {
		Init();
		LoopDevice::LoopDevice(int fd, const std::chrono::milliseconds& timeout_ms, bool auto_close)
		: fd_(fd), owns_fd_(auto_close) {
		Init(timeout_ms);
		}

		LoopDevice::LoopDevice(const std::string& path) : fd_(-1), owns_fd_(true) {
		LoopDevice::LoopDevice(const std::string& path, const std::chrono::milliseconds& timeout_ms)
		: fd_(-1), owns_fd_(true) {
		fd_.reset(open(path.c_str(), O_RDWR \| O_CLOEXEC));
		if (fd_ < -1) {
		PLOG(ERROR) << "open failed for " << path;
		return;
		}
		Init();
		Init(timeout_ms);
		}

		LoopDevice::~LoopDevice() {
		@@ -134,8 +157,8 @@ LoopDevice::~LoopDevice() {
		}
		}

		void LoopDevice::Init() {
		control_.Attach(fd_, &device_);
		void LoopDevice::Init(const std::chrono::milliseconds& timeout_ms) {
		valid_ = control_.Attach(fd_, timeout_ms, &device_);
		}

		} // namespace dm