Better handle invalid read-write mounts

PiperOrigin-RevId: 433136095
Change-Id: I17eb347c0a5cfef5e05c3717dfdd83055d967e35
This commit is contained in:
Wiktor Garbacz 2022-03-07 23:57:28 -08:00 committed by Copybara-Service
parent 32d19f9e57
commit 8a5740fbb1
2 changed files with 33 additions and 2 deletions

View File

@ -479,6 +479,7 @@ uint64_t GetMountFlagsFor(const std::string& path) {
uint64_t flags = 0;
using MountPair = std::pair<uint64_t, uint64_t>;
for (const auto& [mount_flag, vfs_flag] : {
MountPair(MS_RDONLY, ST_RDONLY),
MountPair(MS_NOSUID, ST_NOSUID),
MountPair(MS_NODEV, ST_NODEV),
MountPair(MS_NOEXEC, ST_NOEXEC),
@ -562,8 +563,14 @@ void MountWithDefaults(const std::string& source, const std::string& target,
// Flags are ignored for a bind mount, a remount is needed to set the flags.
if (extra_flags & MS_BIND) {
// Get actual mount flags.
flags |= GetMountFlagsFor(target);
res = mount("", target.c_str(), "", flags | MS_REMOUNT, nullptr);
uint64_t target_flags = GetMountFlagsFor(target);
if ((target_flags & MS_RDONLY) != 0 && (flags & MS_RDONLY) == 0) {
SAPI_RAW_LOG(FATAL,
"cannot remount %s as read-write as it's on read-only dev",
target.c_str());
}
res = mount("", target.c_str(), "", flags | target_flags | MS_REMOUNT,
nullptr);
SAPI_RAW_PCHECK(res != -1, "remounting %s with flags=%s failed", target,
MountFlagsToString(flags));
}

View File

@ -22,6 +22,7 @@
#include <linux/random.h> // For GRND_NONBLOCK
#include <sys/mman.h> // For mmap arguments
#include <sys/socket.h>
#include <sys/statvfs.h>
#include <syscall.h>
#include <array>
@ -72,6 +73,15 @@ bool CheckBpfBounds(const sock_filter& filter, size_t max_jmp) {
return true;
}
bool IsOnReadOnlyDev(const std::string& path) {
struct statvfs vfs;
if (TEMP_FAILURE_RETRY(statvfs(path.c_str(), &vfs)) == -1) {
PLOG(ERROR) << "Could not statvfs: " << path.c_str();
return false;
}
return vfs.f_flag & ST_RDONLY;
}
} // namespace
PolicyBuilder& PolicyBuilder::AllowSyscall(uint32_t num) {
@ -939,6 +949,13 @@ PolicyBuilder& PolicyBuilder::AddFileAt(absl::string_view outside,
return *this;
}
if (!is_ro && IsOnReadOnlyDev(*valid_outside)) {
SetError(absl::FailedPreconditionError(
absl::StrCat("Cannot add ", outside,
" as read-write as it's on a read-only device")));
return *this;
}
if (auto status = mounts_.AddFileAt(*valid_outside, inside, is_ro);
!status.ok()) {
SetError(
@ -995,6 +1012,13 @@ PolicyBuilder& PolicyBuilder::AddDirectoryAt(absl::string_view outside,
return *this;
}
if (!is_ro && IsOnReadOnlyDev(*valid_outside)) {
SetError(absl::FailedPreconditionError(
absl::StrCat("Cannot add ", outside,
" as read-write as it's on a read-only device")));
return *this;
}
if (absl::Status status =
mounts_.AddDirectoryAt(*valid_outside, inside, is_ro);
!status.ok()) {