update_engine: Handle p2p file sharing `size_t` overflow
Also create interface to allow for finer grain testing.
BUG=b:227282649
TEST=FEATURES=test emerge-$B update_engine
TEST=p2p file sharing
Change-Id: I81361299577d748e69983f5ce0564529ff47e1b2
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/3633019
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Yuanpeng Ni <yuanpengni@chromium.org>
Reviewed-by: Henry Barnor <hbarnor@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index 4041451..568c923 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -181,6 +181,7 @@
static_library("libupdate_engine") {
sources = [
"certificate_checker.cc",
+ "common/call_wrapper.cc",
"common/connection_utils.cc",
"common/system_state.cc",
"cros/boot_control_chromeos.cc",
diff --git a/common/call_wrapper.cc b/common/call_wrapper.cc
new file mode 100644
index 0000000..844461c
--- /dev/null
+++ b/common/call_wrapper.cc
@@ -0,0 +1,34 @@
+//
+// Copyright (C) 2022 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 "update_engine/common/call_wrapper.h"
+
+#include <memory>
+
+#include <base/files/file_path.h>
+#include <base/system/sys_info.h>
+
+namespace chromeos_update_engine {
+
+std::unique_ptr<CallWrapperInterface> CreateCallWrapper() {
+ return std::make_unique<CallWrapper>();
+}
+
+int64_t CallWrapper::AmountOfFreeDiskSpace(const base::FilePath& path) {
+ return base::SysInfo::AmountOfFreeDiskSpace(path);
+}
+
+} // namespace chromeos_update_engine
diff --git a/common/call_wrapper.h b/common/call_wrapper.h
new file mode 100644
index 0000000..bb24774
--- /dev/null
+++ b/common/call_wrapper.h
@@ -0,0 +1,40 @@
+//
+// Copyright (C) 2022 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 UPDATE_ENGINE_COMMON_CALL_WRAPPER_H_
+#define UPDATE_ENGINE_COMMON_CALL_WRAPPER_H_
+
+#include "update_engine/common/call_wrapper_interface.h"
+
+#include <base/files/file_path.h>
+
+namespace chromeos_update_engine {
+
+class CallWrapper : public CallWrapperInterface {
+ public:
+ CallWrapper() = default;
+ CallWrapper(const CallWrapper&) = delete;
+ CallWrapper& operator=(const CallWrapper&) = delete;
+
+ ~CallWrapper() = default;
+
+ // CallWrapperInterface overrides.
+ int64_t AmountOfFreeDiskSpace(const base::FilePath& path) override;
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_COMMON_CALL_WRAPPER_H_
diff --git a/common/call_wrapper_interface.h b/common/call_wrapper_interface.h
new file mode 100644
index 0000000..9eb22b5
--- /dev/null
+++ b/common/call_wrapper_interface.h
@@ -0,0 +1,47 @@
+//
+// Copyright (C) 2022 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 UPDATE_ENGINE_COMMON_CALL_WRAPPER_INTERFACE_H_
+#define UPDATE_ENGINE_COMMON_CALL_WRAPPER_INTERFACE_H_
+
+#include <memory>
+
+#include <base/files/file_path.h>
+
+namespace chromeos_update_engine {
+
+// The abstract call_wrapper interface used mainly to intercept libc or system
+// calls for testing.
+class CallWrapperInterface {
+ public:
+ CallWrapperInterface(const CallWrapperInterface&) = delete;
+ CallWrapperInterface& operator=(const CallWrapperInterface&) = delete;
+
+ virtual ~CallWrapperInterface() = default;
+
+ virtual int64_t AmountOfFreeDiskSpace(const base::FilePath& path) = 0;
+
+ protected:
+ CallWrapperInterface() = default;
+};
+
+// This factory function creates a new CallWrapperInterface instance for the
+// current platform.
+std::unique_ptr<CallWrapperInterface> CreateCallWrapper();
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_COMMON_CALL_WRAPPER_INTERFACE_H_
diff --git a/common/mock_call_wrapper.h b/common/mock_call_wrapper.h
new file mode 100644
index 0000000..65a5705
--- /dev/null
+++ b/common/mock_call_wrapper.h
@@ -0,0 +1,36 @@
+//
+// Copyright (C) 2022 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 UPDATE_ENGINE_COMMON_MOCK_CALL_WRAPPER_H_
+#define UPDATE_ENGINE_COMMON_MOCK_CALL_WRAPPER_H_
+
+#include "update_engine/common/call_wrapper_interface.h"
+
+#include <gmock/gmock.h>
+
+namespace chromeos_update_engine {
+
+class MockCallWrapper : public CallWrapperInterface {
+ public:
+ MOCK_METHOD(int64_t,
+ AmountOfFreeDiskSpace,
+ (const base::FilePath& path),
+ (override));
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_COMMON_MOCK_CALL_WRAPPER_H_
diff --git a/common/system_state.h b/common/system_state.h
index 703e9b1..4fc1aac 100644
--- a/common/system_state.h
+++ b/common/system_state.h
@@ -52,6 +52,7 @@
class PayloadStateInterface;
class PowerManagerInterface;
class UpdateAttempter;
+class CallWrapperInterface;
// An interface to global system context, including platform resources,
// the current state of the system, high-level objects whose lifetime is same
@@ -124,6 +125,9 @@
// Returns a pointer to the CrosHealthdInteraface singleton.
virtual CrosHealthdInterface* cros_healthd() = 0;
+ // Returns a pointer to the CallWrapperInterface singleton.
+ virtual CallWrapperInterface* call_wrapper() = 0;
+
protected:
static SystemState* g_pointer_;
};
diff --git a/cros/download_action_chromeos_unittest.cc b/cros/download_action_chromeos_unittest.cc
index cf46c9c..6567084 100644
--- a/cros/download_action_chromeos_unittest.cc
+++ b/cros/download_action_chromeos_unittest.cc
@@ -546,6 +546,9 @@
EXPECT_CALL(*FakeSystemState::Get()->mock_payload_state(),
GetUsingP2PForSharing())
.WillRepeatedly(Return(use_p2p_to_share));
+ EXPECT_CALL(*FakeSystemState::Get()->mock_call_wrapper(),
+ AmountOfFreeDiskSpace(_))
+ .WillRepeatedly(Return(data_.length() * 2));
ScopedTempFile output_temp_file;
TestDirectFileWriter writer;
@@ -635,6 +638,9 @@
TEST_F(P2PDownloadActionTest, CanAppend) {
SetupDownload(1000); // starting_offset
+ EXPECT_CALL(*FakeSystemState::Get()->mock_call_wrapper(),
+ AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(data_.length() * 2));
// Prepare the file with existing data before starting to write to
// it via DownloadAction.
@@ -669,6 +675,9 @@
TEST_F(P2PDownloadActionTest, DeletePartialP2PFileIfResumingWithoutP2P) {
SetupDownload(1000); // starting_offset
+ EXPECT_CALL(*FakeSystemState::Get()->mock_call_wrapper(),
+ AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(data_.length() * 2));
// Prepare the file with all existing data before starting to write
// to it via DownloadAction.
@@ -699,6 +708,9 @@
EXPECT_CALL(*FakeSystemState::Get()->mock_payload_state(),
GetUsingP2PForSharing())
.WillRepeatedly(Return(true));
+ EXPECT_CALL(*FakeSystemState::Get()->mock_call_wrapper(),
+ AmountOfFreeDiskSpace(_))
+ .WillRepeatedly(Return(data_.length() * 2));
EXPECT_CALL(*FakeSystemState::Get()->mock_payload_state(), NextPayload())
.WillOnce(Return(true));
diff --git a/cros/fake_system_state.cc b/cros/fake_system_state.cc
index 7673b1d..8ca424d 100644
--- a/cros/fake_system_state.cc
+++ b/cros/fake_system_state.cc
@@ -33,6 +33,7 @@
request_params_(&mock_request_params_),
p2p_manager_(&mock_p2p_manager_),
update_manager_(&fake_update_manager_),
+ call_wrapper_(&mock_call_wrapper_),
device_policy_(nullptr),
fake_system_rebooted_(false) {
mock_payload_state_.Initialize();
diff --git a/cros/fake_system_state.h b/cros/fake_system_state.h
index 652a3bc..2446895 100644
--- a/cros/fake_system_state.h
+++ b/cros/fake_system_state.h
@@ -28,6 +28,7 @@
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/fake_hardware.h"
#include "update_engine/common/fake_prefs.h"
+#include "update_engine/common/mock_call_wrapper.h"
#include "update_engine/common/mock_metrics_reporter.h"
#include "update_engine/common/mock_prefs.h"
#include "update_engine/common/system_state.h"
@@ -115,6 +116,8 @@
inline CrosHealthdInterface* cros_healthd() override { return cros_healthd_; }
+ inline CallWrapperInterface* call_wrapper() override { return call_wrapper_; }
+
inline bool system_rebooted() override { return fake_system_rebooted_; }
// Setters for the various members, can be used for overriding the default
@@ -188,6 +191,15 @@
cros_healthd_ = cros_healthd;
}
+ inline void set_call_wrapper(CallWrapperInterface* call_wrapper) {
+ call_wrapper_ = call_wrapper;
+ }
+
+ inline testing::StrictMock<MockCallWrapper>* mock_call_wrapper() {
+ CHECK(call_wrapper_ == &mock_call_wrapper_);
+ return &mock_call_wrapper_;
+ }
+
// Getters for the built-in default implementations. These return the actual
// concrete type of each implementation. For additional safety, they will fail
// whenever the requested default was overridden by a different
@@ -284,6 +296,7 @@
testing::NiceMock<MockOmahaRequestParams> mock_request_params_;
testing::NiceMock<MockP2PManager> mock_p2p_manager_;
testing::NiceMock<MockPowerManager> mock_power_manager_;
+ testing::StrictMock<MockCallWrapper> mock_call_wrapper_;
// Pointers to objects that client code can override. They are initialized to
// the default implementations above.
@@ -302,6 +315,7 @@
PowerManagerInterface* power_manager_{&mock_power_manager_};
DlcServiceInterface* dlcservice_;
CrosHealthdInterface* cros_healthd_;
+ CallWrapperInterface* call_wrapper_;
// Other object pointers (not preinitialized).
const policy::DevicePolicy* device_policy_;
diff --git a/cros/p2p_manager.cc b/cros/p2p_manager.cc
index abf894c..40f23a4 100644
--- a/cros/p2p_manager.cc
+++ b/cros/p2p_manager.cc
@@ -50,6 +50,7 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
+#include "update_engine/common/call_wrapper.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/system_state.h"
#include "update_engine/common/utils.h"
@@ -514,15 +515,16 @@
// Before creating the file, bail if statvfs(3) indicates that at
// least twice the size is not available in P2P_DIR.
- struct statvfs statvfsbuf;
FilePath p2p_dir = configuration_->GetP2PDir();
- if (statvfs(p2p_dir.value().c_str(), &statvfsbuf) != 0) {
- PLOG(ERROR) << "Error calling statvfs() for dir " << p2p_dir.value();
+ int64_t free_bytes =
+ SystemState::Get()->call_wrapper()->AmountOfFreeDiskSpace(p2p_dir);
+ if (free_bytes < 0) {
+ PLOG(ERROR) << "Error getting amount of free disk space from "
+ << p2p_dir.value();
return false;
}
- size_t free_bytes =
- static_cast<size_t>(statvfsbuf.f_bsize) * statvfsbuf.f_bavail;
- if (free_bytes < 2 * expected_size) {
+ // Compare sizes this way to handle overflows.
+ if (free_bytes / 2 < expected_size) {
// This can easily happen and is worth reporting.
LOG(INFO) << "Refusing to allocate p2p file of " << expected_size
<< " bytes since the directory " << p2p_dir.value()
diff --git a/cros/p2p_manager_unittest.cc b/cros/p2p_manager_unittest.cc
index 1a7d1ee..f822027 100644
--- a/cros/p2p_manager_unittest.cc
+++ b/cros/p2p_manager_unittest.cc
@@ -49,6 +49,7 @@
#include <policy/libpolicy.h>
#include <policy/mock_device_policy.h>
+#include "update_engine/common/mock_call_wrapper.h"
#include "update_engine/common/prefs.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/test_utils.h"
@@ -79,6 +80,7 @@
FakeSystemState::CreateInstance();
test_conf_ = new FakeP2PManagerConfiguration();
fake_um_ = FakeSystemState::Get()->fake_update_manager();
+ mock_call_wrapper_ = FakeSystemState::Get()->mock_call_wrapper();
// Construct the P2P manager under test.
manager_.reset(P2PManager::Construct(
@@ -94,6 +96,8 @@
chromeos_update_manager::FakeUpdateManager* fake_um_;
unique_ptr<P2PManager> manager_;
+
+ MockCallWrapper* mock_call_wrapper_;
};
// Check that |IsP2PEnabled()| polls the policy correctly, with the value not
@@ -357,6 +361,8 @@
// Check that sharing a *new* file works.
TEST_F(P2PManagerTest, ShareFile) {
const int kP2PTestFileSize = 1000 * 8; // 8 KB
+ EXPECT_CALL(*mock_call_wrapper_, AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(kP2PTestFileSize * 2));
EXPECT_TRUE(manager_->FileShare("foo", kP2PTestFileSize));
EXPECT_EQ(manager_->FileGetPath("foo"),
@@ -376,6 +382,8 @@
// Check that making a shared file visible, does what is expected.
TEST_F(P2PManagerTest, MakeFileVisible) {
const int kP2PTestFileSize = 1000 * 8; // 8 KB
+ EXPECT_CALL(*mock_call_wrapper_, AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(kP2PTestFileSize * 2));
// First, check that it's not visible.
manager_->FileShare("foo", kP2PTestFileSize);
@@ -398,6 +406,20 @@
}
}
+TEST_F(P2PManagerTest, SharingFileBytesMoreThanNecessaryStorageSpace) {
+ const int kP2PTestFileSize = 16 * (1 << 10); // 16 KB
+ EXPECT_CALL(*mock_call_wrapper_, AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(kP2PTestFileSize + 1));
+ EXPECT_FALSE(manager_->FileShare("foo", kP2PTestFileSize));
+}
+
+TEST_F(P2PManagerTest, SharingFileBytesLessThanNecessaryStorageSpace) {
+ const int kP2PTestFileSize = 16 * (1 << 10); // 16 KB
+ EXPECT_CALL(*mock_call_wrapper_, AmountOfFreeDiskSpace(_))
+ .WillOnce(Return(kP2PTestFileSize * 2));
+ EXPECT_TRUE(manager_->FileShare("foo", kP2PTestFileSize));
+}
+
// Check that we return the right values for existing files in P2P_DIR.
TEST_F(P2PManagerTest, ExistingFiles) {
bool visible;
diff --git a/cros/real_system_state.cc b/cros/real_system_state.cc
index 552b5bc..8e8d0fd 100644
--- a/cros/real_system_state.cc
+++ b/cros/real_system_state.cc
@@ -84,6 +84,12 @@
return false;
}
+ call_wrapper_ = CreateCallWrapper();
+ if (!call_wrapper_) {
+ LOG(ERROR) << "Error initializing the CallWrapperInterface.";
+ return false;
+ }
+
// Initialize standard and powerwash-safe prefs.
base::FilePath non_volatile_path;
// TODO(deymo): Fall back to in-memory prefs if there's no physical directory
diff --git a/cros/real_system_state.h b/cros/real_system_state.h
index 2c0362b..1c7ea7b 100644
--- a/cros/real_system_state.h
+++ b/cros/real_system_state.h
@@ -27,6 +27,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/common/boot_control_interface.h"
+#include "update_engine/common/call_wrapper_interface.h"
#include "update_engine/common/clock.h"
#include "update_engine/common/cros_healthd_interface.h"
#include "update_engine/common/daemon_state_interface.h"
@@ -115,6 +116,8 @@
CrosHealthdInterface* cros_healthd() override { return cros_healthd_.get(); }
+ CallWrapperInterface* call_wrapper() override { return call_wrapper_.get(); }
+
private:
// Initializes and sets systems objects that require an initialization
// separately from construction. Returns |true| on success.
@@ -178,6 +181,9 @@
policy::PolicyProvider policy_provider_;
+ // Interface for cros_healthd.
+ std::unique_ptr<CallWrapperInterface> call_wrapper_;
+
// If true, this is the first instance of the update engine since the system
// rebooted. Important for tracking whether you are running instance of the
// update engine on first boot or due to a crash/restart.
