0

Simplify sandbox IPC service count

kMaxServiceCount is needlessly larger than it needs
to be and can be directly coupled to the number of IPCs
supported. This CL renames it to kSandboxIpcCount and
defines it by reference to the IpcTag enum.

(History: kMaxServiceCount was last increased in
https://codereview.chromium.org/1856993003.)

Bug: 336349858
Change-Id: Idcc690c22667649faa5614b36d595e12f59e1b34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491721
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294191}
This commit is contained in:
Alex Gough
2024-04-30 07:42:55 +00:00
committed by Chromium LUCI CQ
parent 71a8e0fbad
commit 6d18fd4823
12 changed files with 24 additions and 25 deletions

@ -112,7 +112,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
// We send the fuzzer generated data to every available policy rule.
// Only some of the services will be registered, but it will
// quickly skip those that have nothing registered.
for (size_t i = 0; i < sandbox::kMaxIpcTag; i++) {
for (size_t i = 0; i < sandbox::kSandboxIpcCount; i++) {
policy->EvalPolicy(static_cast<sandbox::IpcTag>(i), real_params);
}

@ -44,7 +44,7 @@
// return codes indicate that the IPC transport failed to deliver it.
namespace sandbox {
enum class IpcTag;
enum class IpcTag : uint32_t;
// this is the assumed channel size. This can be overridden in a given
// IPC implementation.

@ -134,8 +134,7 @@ PolicyGlobal* GenerateBlankPolicy() {
LowLevelPolicy policy_maker(policy);
for (int i = static_cast<int>(IpcTag::UNUSED);
i < static_cast<int>(IpcTag::LAST); i++) {
for (size_t i = 0; i < kSandboxIpcCount; i++) {
IpcTag service = static_cast<IpcTag>(i);
PolicyRule ask_broker(ASK_BROKER);
ask_broker.Done();
@ -157,7 +156,7 @@ void CopyPolicyToTarget(const void* source, size_t size, void* dest) {
size_t offset = reinterpret_cast<size_t>(source);
for (size_t i = 0; i < sandbox::kMaxServiceCount; i++) {
for (size_t i = 0; i < kSandboxIpcCount; i++) {
size_t buffer = reinterpret_cast<size_t>(policy->entry[i]);
if (buffer) {
buffer -= offset;

@ -5,10 +5,12 @@
#ifndef SANDBOX_WIN_SRC_IPC_TAGS_H_
#define SANDBOX_WIN_SRC_IPC_TAGS_H_
#include <cstdint>
namespace sandbox {
enum class IpcTag {
UNUSED = 0,
enum class IpcTag : uint32_t {
UNUSED,
PING1, // Takes a cookie in parameters and returns the cookie
// multiplied by 2 and the tick_count. Used for testing only.
PING2, // Takes an in/out cookie in parameters and modify the cookie
@ -25,13 +27,12 @@ enum class IpcTag {
USER_REGISTERCLASSW,
CREATETHREAD,
NTCREATESECTION,
LAST
kMaxValue = NTCREATESECTION,
};
constexpr size_t kMaxServiceCount = 64;
constexpr size_t kMaxIpcTag = static_cast<size_t>(IpcTag::LAST);
static_assert(kMaxIpcTag <= kMaxServiceCount, "kMaxServiceCount is too low");
// The number of IpcTag services that are defined.
inline constexpr size_t kSandboxIpcCount =
static_cast<size_t>(IpcTag::kMaxValue) + 1;
} // namespace sandbox
#endif // SANDBOX_WIN_SRC_IPC_TAGS_H_

@ -85,7 +85,7 @@ bool LowLevelPolicy::Done() {
for (Mmap::iterator it = mmap.begin(); it != mmap.end(); ++it) {
IpcTag service = (*it).first;
if (static_cast<size_t>(service) >= kMaxServiceCount) {
if (service > IpcTag::kMaxValue) {
return false;
}
policy_store_->entry[static_cast<size_t>(service)] = current_buffer;

@ -75,7 +75,7 @@ struct PolicyGlobal {
return entry[static_cast<size_t>(service)] != nullptr;
}
PolicyBuffer* entry[kMaxServiceCount];
PolicyBuffer* entry[kSandboxIpcCount];
size_t data_size;
PolicyBuffer data[1];
};

@ -24,10 +24,11 @@ extern void* volatile g_shared_policy_memory;
SANDBOX_INTERCEPT size_t g_shared_policy_size;
bool QueryBroker(IpcTag ipc_id, CountedParameterSetBase* params) {
DCHECK_NT(static_cast<size_t>(ipc_id) < kMaxServiceCount);
DCHECK_NT(ipc_id <= IpcTag::kMaxValue);
if (static_cast<size_t>(ipc_id) >= kMaxServiceCount)
if (ipc_id <= IpcTag::UNUSED || ipc_id > IpcTag::kMaxValue) {
return false;
}
// Policy is only sent if required.
if (!g_shared_policy_memory) {

@ -751,7 +751,7 @@ ResultCode PolicyBase::SetupAllInterceptions(TargetProcess& target) {
InterceptionManager manager(target);
PolicyGlobal* policy = config()->policy();
if (policy) {
for (size_t i = 0; i < kMaxIpcTag; i++) {
for (size_t i = 0; i < kSandboxIpcCount; i++) {
if (policy->entry[i] &&
!dispatcher_->SetupService(&manager, static_cast<IpcTag>(i)))
return SBOX_ERROR_SETUP_INTERCEPTION_SERVICE;

@ -181,9 +181,6 @@ std::string GetIpcTagAsString(IpcTag service) {
return "CreateThread";
case IpcTag::NTCREATESECTION:
return "NtCreateSection";
case IpcTag::LAST:
DCHECK(false) << "Unknown IpcTag";
return "Unknown";
}
}
@ -403,7 +400,7 @@ PolicyDiagnostic::PolicyDiagnostic(PolicyBase* policy) {
// Fixup pointers (see |PolicyGlobal| in policy_low_level.h).
PolicyBuffer** original_entries = original_rules->entry;
PolicyBuffer** copy_base = policy_rules_->entry;
for (size_t i = 0; i < kMaxServiceCount; i++) {
for (size_t i = 0; i < kSandboxIpcCount; i++) {
if (policy_rules_->entry[i]) {
policy_rules_->entry[i] = reinterpret_cast<PolicyBuffer*>(
reinterpret_cast<char*>(copy_base) +

@ -67,7 +67,7 @@ void CopyPolicyToTarget(base::span<const uint8_t> source, void* dest) {
size_t offset = reinterpret_cast<size_t>(source.data());
for (size_t i = 0; i < sandbox::kMaxServiceCount; i++) {
for (size_t i = 0; i < sandbox::kSandboxIpcCount; i++) {
size_t buffer = reinterpret_cast<size_t>(policy->entry[i]);
if (buffer) {
buffer -= offset;

@ -78,7 +78,7 @@ TopLevelDispatcher::~TopLevelDispatcher() {}
std::vector<IpcTag> TopLevelDispatcher::ipc_targets() {
std::vector<IpcTag> results = {IpcTag::PING1, IpcTag::PING2};
for (uint32_t ipc = 0; ipc < kMaxIpcTag; ipc++) {
for (size_t ipc = 0; ipc < kSandboxIpcCount; ipc++) {
if (ipc_targets_[ipc]) {
results.push_back(static_cast<IpcTag>(ipc));
}
@ -151,8 +151,9 @@ bool TopLevelDispatcher::Ping(IPCInfo* ipc, void* arg1) {
}
Dispatcher* TopLevelDispatcher::GetDispatcher(IpcTag ipc_tag) {
if (ipc_tag >= IpcTag::LAST || ipc_tag <= IpcTag::UNUSED)
if (ipc_tag > IpcTag::kMaxValue || ipc_tag == IpcTag::UNUSED) {
return nullptr;
}
return ipc_targets_[static_cast<size_t>(ipc_tag)];
}

@ -49,7 +49,7 @@ class TopLevelDispatcher : public Dispatcher {
std::unique_ptr<Dispatcher> handle_dispatcher_;
std::unique_ptr<Dispatcher> process_mitigations_win32k_dispatcher_;
std::unique_ptr<Dispatcher> signed_dispatcher_;
Dispatcher* ipc_targets_[kMaxIpcTag];
Dispatcher* ipc_targets_[kSandboxIpcCount];
};
} // namespace sandbox