Skip to content

Commit 29e9c41

Browse files
happyCoder92copybara-github
authored andcommitted
Apply the rlimits after sending over the unotify FD
Unfortunately Linux kernel accounts all FDs inflight in unix socket to the user (uid). When starting multiple sandboxees concurrently with a low RLIMIT_NOFILE we can go over that limit on send (checks against sum of open fds for the process and inflight fds for the user). We should therefore avoid any SendFDs on setup path after the limits are applied. PiperOrigin-RevId: 897157290 Change-Id: I71ab1568e7f363d741716a99782a5db720052355
1 parent 5c6eab5 commit 29e9c41

6 files changed

Lines changed: 39 additions & 10 deletions

File tree

sandboxed_api/sandbox2/client.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ void InitSeccompUnotify(sock_fprog prog, Comms* comms,
9494
error.store(true, std::memory_order_seq_cst);
9595
SAPI_RAW_LOG(FATAL, "closing unotify fd");
9696
}
97+
// Wait for the monitor to apply limits.
98+
uint32_t msg;
99+
if (!comms->RecvUint32(&msg)) {
100+
error.store(true, std::memory_order_seq_cst);
101+
SAPI_RAW_LOG(FATAL, "receiving limits applied message");
102+
}
103+
if (msg != Client::kSandbox2UnotifyLimitsApplied) {
104+
error.store(true, std::memory_order_seq_cst);
105+
SAPI_RAW_LOG(FATAL, "invalid limits applied message");
106+
}
97107
sock_filter filter = ALLOW;
98108
struct sock_fprog allow_prog = {
99109
.len = 1,

sandboxed_api/sandbox2/client.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ class Client {
4545
// UnotifyMonitor.
4646
static constexpr uint32_t kSandbox2ClientUnotify = 0x0A0B0C03;
4747

48+
// Sandboxee can proceed after seccomp_unotify setup as limits have been
49+
// applied by the monitor.
50+
static constexpr uint32_t kSandbox2UnotifyLimitsApplied = 0x0A0B0C04;
51+
4852
// Allow speculation in the seccomp policy.
4953
static constexpr uint32_t kAllowSpeculationBit = 0x10000000;
5054

sandboxed_api/sandbox2/monitor_base.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,6 @@ void MonitorBase::Launch() {
236236
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_WAIT);
237237
return;
238238
}
239-
if (!InitApplyLimits()) {
240-
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_LIMITS);
241-
return;
242-
}
243239
std::move(process_cleanup).Cancel();
244240

245241
RunInternal();

sandboxed_api/sandbox2/monitor_base.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ class MonitorBase {
8181
virtual void SetWallTimeLimit(absl::Duration limit) = 0;
8282

8383
protected:
84+
// Applies limits on the sandboxee.
85+
bool InitApplyLimits();
86+
8487
// Sends the policy to the client.
8588
// Can be overridden by subclasses to save/modify policy before sending.
8689
// Returns success/failure status.
@@ -153,9 +156,6 @@ class MonitorBase {
153156
// Sends information about the current working directory.
154157
bool InitSendCwd();
155158

156-
// Applies limits on the sandboxee.
157-
bool InitApplyLimits();
158-
159159
// Applies individual limit on the sandboxee.
160160
bool InitApplyLimit(pid_t pid, int resource, const rlimit64& rlim) const;
161161

sandboxed_api/sandbox2/monitor_ptrace.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,18 @@ void PtraceMonitor::Run() {
315315
getrusage(RUSAGE_THREAD, result_.GetRUsageMonitor());
316316
OnDone();
317317
};
318-
319318
absl::Cleanup setup_notify = [this] { setup_notification_.Notify(); };
319+
absl::Cleanup process_cleanup = [this] {
320+
if (process_.init_pid > 0) {
321+
kill(process_.init_pid, SIGKILL);
322+
} else if (process_.main_pid > 0) {
323+
kill(process_.main_pid, SIGKILL);
324+
}
325+
};
326+
if (!InitApplyLimits()) {
327+
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_LIMITS);
328+
return;
329+
}
320330
// It'd be costly to initialize the sigset_t for each sigtimedwait()
321331
// invocation, so do it once per Monitor.
322332
if (!use_deadline_manager_ && !InitSetupSignals()) {
@@ -334,6 +344,7 @@ void PtraceMonitor::Run() {
334344
// Tell the parent thread (Sandbox2 object) that we're done with the initial
335345
// set-up process of the sandboxee.
336346
std::move(setup_notify).Invoke();
347+
std::move(process_cleanup).Cancel();
337348

338349
bool sandboxee_exited = false;
339350
pid_waiter_.SetPriorityPid(process_.main_pid);

sandboxed_api/sandbox2/monitor_unotify.cc

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "sandboxed_api/sandbox2/monitor_unotify.h"
22

3+
#include <fcntl.h>
34
#include <linux/seccomp.h>
45
#include <poll.h>
56
#include <sys/eventfd.h>
@@ -259,12 +260,20 @@ void UnotifyMonitor::Run() {
259260
getrusage(RUSAGE_THREAD, result_.GetRUsageMonitor());
260261
OnDone();
261262
};
262-
263263
absl::Cleanup setup_notify = [this] { setup_notification_.Notify(); };
264+
absl::Cleanup process_cleanup = [this] { KillInit(); };
264265
if (!InitSetupUnotify()) {
265266
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
266267
return;
267268
}
269+
if (!InitApplyLimits()) {
270+
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_LIMITS);
271+
return;
272+
}
273+
if (!comms_->SendUint32(Client::kSandbox2UnotifyLimitsApplied)) {
274+
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
275+
return;
276+
}
268277
if (!InitSetupNotifyEventFd()) {
269278
SetExitStatusCode(Result::SETUP_ERROR, Result::FAILED_NOTIFY);
270279
return;
@@ -340,7 +349,6 @@ void UnotifyMonitor::Run() {
340349
HandleUnotify();
341350
}
342351
}
343-
KillInit();
344352
}
345353

346354
void UnotifyMonitor::SetExitStatusFromStatusPipe() {

0 commit comments

Comments
 (0)