Skip to content

Commit bb0c441

Browse files
committed
Merged PR 692773: Properly invalidate the file descriptor table on fork/clone
Some other cases where we were not doing the proper file descriptor cache invalidation: * Fork/clone: the entire fd table needs to be invalidated for the child process since fd are local to a process * Internal open handles (sandbox sending reports/logging) also need to clean up their corresponding entries as a defense for missing close(fd),
1 parent 95cc00e commit bb0c441

File tree

3 files changed

+40
-3
lines changed

3 files changed

+40
-3
lines changed

Public/Src/Sandbox/Linux/bxl_observer.cpp

+14
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,13 @@ bool BxlObserver::Send(const char *buf, size_t bufsiz)
203203
_fatal("Wrote only %ld bytes out of %ld", numWritten, bufsiz);
204204
}
205205

206+
// A handle was opened for our own internal purposes. That
207+
// could have reused a fd where we missed a close,
208+
// so reset that entry in the fd table
209+
reset_fd_table_entry(logFd);
210+
206211
real_close(logFd);
212+
207213
return true;
208214
}
209215

@@ -376,6 +382,14 @@ void BxlObserver::reset_fd_table_entry(int fd)
376382
}
377383
}
378384

385+
void BxlObserver::reset_fd_table()
386+
{
387+
for (int i = 0; i < MAX_FD; i++)
388+
{
389+
fdTable_[i] = empty_str_;
390+
}
391+
}
392+
379393
std::string BxlObserver::fd_to_path(int fd)
380394
{
381395
char path[PATH_MAX] = {0};

Public/Src/Sandbox/Linux/bxl_observer.hpp

+9
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,10 @@ class BxlObserver final
298298
// and the write is allowed by policy
299299
AccessCheckResult report_firstAllowWriteCheck(const char *fullPath);
300300

301+
// Clears the specified entry on the file descriptor table
301302
void reset_fd_table_entry(int fd);
303+
// Clears the entire file descriptor table
304+
void reset_fd_table();
302305
std::string fd_to_path(int fd);
303306
std::string normalize_path_at(int dirfd, const char *pathname, int oflags = 0);
304307

@@ -321,6 +324,12 @@ class BxlObserver final
321324
va_start(args, fmt);
322325
real_vfprintf(f, fmt, args);
323326
va_end(args);
327+
328+
// A handle was opened for our own internal purposes. That
329+
// could have reused a fd where we missed a close,
330+
// so reset that entry in the fd table
331+
reset_fd_table_entry(fileno(f));
332+
324333
real_fclose(f);
325334
}
326335
}

Public/Src/Sandbox/Linux/detours.cpp

+17-3
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,15 @@ static int ret_fd(int fd, BxlObserver *bxl)
5353
INTERPOSE(pid_t, fork, void)({
5454
result_t<pid_t> childPid = bxl->fwd_fork();
5555

56-
// report fork only when we are in the parent process
57-
if (childPid.get() > 0)
56+
if (childPid.get() == 0)
5857
{
58+
// Clear the file descriptor table when we are in the child process
59+
// File descriptors are unique to a process, so this cache needs to be invalidated on the child
60+
bxl->reset_fd_table();
61+
}
62+
else
63+
{
64+
// report fork only when we are in the parent process
5965
report_child_process(__func__, bxl, childPid.get());
6066
}
6167

@@ -71,8 +77,16 @@ INTERPOSE(int, clone, int (*fn)(void *), void *child_stack, int flags, void *arg
7177
va_end(args);
7278

7379
result_t<int> result = bxl->fwd_clone(fn, child_stack, flags, arg, ptid, newtls, ctid);
74-
if (result.get() > 0)
80+
81+
if (result.get() == 0)
82+
{
83+
// Clear the file descriptor table when we are in the child process
84+
// File descriptors are unique to a process, so this cache needs to be invalidated on the child
85+
bxl->reset_fd_table();
86+
}
87+
else
7588
{
89+
// report clone only when we are in the parent process
7690
report_child_process(__func__, bxl, result.get());
7791
}
7892

0 commit comments

Comments
 (0)