Merged PR 785519: Introduce internal_ variants preserving errno for robustness while doing internal sandbox operations

We should only use `real_` when we are very sure that it's okay to modify errno (for example, during sending we know that we will restore errno at the end) or if we want to examine `errno` of that operation ourselves (but we don't have instances of this in the codebase today). Let's try to use `internal_`, which automatically restores errno, whenever in doubt

Related work items: #2180657
This commit is contained in:
Marcelo Lynch 🧉 2024-05-17 23:16:52 +00:00
Родитель 1c036b601f
Коммит e6f832a249
2 изменённых файлов: 44 добавлений и 31 удалений

Просмотреть файл

@ -35,7 +35,7 @@ BxlObserver::BxlObserver()
}
else
{
real_readlink("/proc/self/exe", progFullPath_, PATH_MAX);
internal_readlink("/proc/self/exe", progFullPath_, PATH_MAX);
}
disposed_ = false;
@ -89,7 +89,7 @@ BxlObserver::~BxlObserver()
{
// best effort, no need to observe the return value here
// if this does fail for whatever reason, the managed side should still unlink this semaphore
real_sem_close(messageCountingSemaphore_);
internal_sem_close(messageCountingSemaphore_);
messageCountingSemaphore_ = nullptr;
}
@ -124,7 +124,7 @@ void BxlObserver::InitFam(pid_t pid)
strlcpy(famPath_, famPath, PATH_MAX);
// read FAM
FILE *famFile = real_fopen(famPath_, "rb");
FILE *famFile = internal_fopen(famPath_, "rb");
if (!famFile)
{
_fatal("Could not open file '%s'; errno: %d", famPath_, errno);
@ -135,8 +135,8 @@ void BxlObserver::InitFam(pid_t pid)
rewind(famFile);
auto famPayload = new char [famLength];
real_fread(famPayload, famLength, 1, famFile);
real_fclose(famFile);
internal_fread(famPayload, famLength, 1, famFile);
internal_fclose(famFile);
// create SandboxedPip (which parses FAM and throws on error)
pip_ = shared_ptr<SandboxedPip>(new SandboxedPip(pid, famPayload, famLength));
@ -164,12 +164,12 @@ void BxlObserver::Init()
{
// Setting initializingSemaphore_ will communicate to the interpose layer to not interpose any libc functions called inside sem_open
initializingSemaphore_ = true;
messageCountingSemaphore_ = real_sem_open(pip_->GetInternalDetoursErrorNotificationFile(), O_CREAT, 0644, 0);
messageCountingSemaphore_ = internal_sem_open(pip_->GetInternalDetoursErrorNotificationFile(), O_CREAT, 0644, 0);
if (messageCountingSemaphore_ == SEM_FAILED)
{
// we'll log a message here, but this won't fail the pip until this feature is tested more thorougly
real_fprintf(stdout, "BuildXL injected message: File access monitoring failed to open message counting semaphore '%s' with errno: '%d'. You should rerun this build, or contact the BuildXL team if the issue persists across multiple builds.", pip_->GetInternalDetoursErrorNotificationFile(), errno);
internal_fprintf(stdout, "BuildXL injected message: File access monitoring failed to open message counting semaphore '%s' with errno: '%d'. You should rerun this build, or contact the BuildXL team if the issue persists across multiple builds.", pip_->GetInternalDetoursErrorNotificationFile(), errno);
}
initializingSemaphore_ = false;
@ -833,9 +833,9 @@ bool BxlObserver::check_and_report_process_requires_ptrace(const char *path)
// If it was changed (has a different modified time), then we should run the check on it once more
struct stat statbuf;
#if (__GLIBC__ == 2 && __GLIBC_MINOR__ < 33)
real___lxstat(1, path, &statbuf);
internal___lxstat(1, path, &statbuf);
#else
real_lstat(path, &statbuf);
internal_lstat(path, &statbuf);
#endif
std::string key = std::to_string(statbuf.st_mtim.tv_sec);
@ -905,7 +905,7 @@ void BxlObserver::set_ptrace_permissions()
bool BxlObserver::is_statically_linked(const char *path)
{
// Before running objdump, lets check if the path exists
if (real_access(path, F_OK) != 0) {
if (internal_access(path, F_OK) != 0) {
return false;
}
@ -1002,7 +1002,7 @@ ssize_t BxlObserver::read_path_for_fd(int fd, char *buf, size_t bufsiz, pid_t as
sprintf(procPath, "/proc/%d/fd/%d", associatedPid, fd);
}
return real_readlink(procPath, buf, bufsiz);
return internal_readlink(procPath, buf, bufsiz);
}
void BxlObserver::reset_fd_table_entry(int fd)
@ -1157,7 +1157,6 @@ void BxlObserver::resolve_path(char *fullpath, bool followFinalSymlink, pid_t as
return;
}
auto prevErrno = errno;
unordered_set<string> visited;
char readlinkBuf[PATH_MAX];
@ -1200,7 +1199,7 @@ void BxlObserver::resolve_path(char *fullpath, bool followFinalSymlink, pid_t as
if (*pFullpath == '/' || (*pFullpath == '\0' && followFinalSymlink))
{
*pFullpath = '\0';
nReadlinkBuf = real_readlink(fullpath, readlinkBuf, PATH_MAX);
nReadlinkBuf = internal_readlink(fullpath, readlinkBuf, PATH_MAX);
*pFullpath = ch;
}
@ -1255,8 +1254,6 @@ void BxlObserver::resolve_path(char *fullpath, bool followFinalSymlink, pid_t as
pFullpath = find_prev_slash(pFullpath);
strcpy(++pFullpath, readlinkBuf);
}
errno = prevErrno;
}
char** BxlObserver::ensure_env_value_with_log(char *const envp[], char const *envName, char const *envValue)
@ -1314,11 +1311,11 @@ bool BxlObserver::EnumerateDirectory(std::string rootDirectory, bool recursive,
auto currentDirectory = directoriesToEnumerate.top();
directoriesToEnumerate.pop();
dir = real_opendir(currentDirectory.c_str());
dir = internal_opendir(currentDirectory.c_str());
if (dir != NULL)
{
while ((ent = real_readdir(dir)) != NULL)
while ((ent = internal_readdir(dir)) != NULL)
{
std::string fileOrDirectory(ent->d_name);
if (fileOrDirectory == "." || fileOrDirectory == "..")
@ -1338,7 +1335,7 @@ bool BxlObserver::EnumerateDirectory(std::string rootDirectory, bool recursive,
filesAndDirectories.push_back(fullPath);
}
real_closedir(dir);
internal_closedir(dir);
}
else
{

Просмотреть файл

@ -49,6 +49,28 @@ static const char GLIBC_23[] = "GLIBC_2.3";
#define ARRAYSIZE(arr) (sizeof(arr)/sizeof(arr[0]))
/*
* real_{fn} provides a way to call the real libc function fn that we interpose.
* Note that calling these functions might modify the global variable errno
* in a meaningful way, so we should proceed with caution when using them
* as part of the operations of the sandbox in the middle of an interposing,
* because the callers of interposed functions might interpret the value of errno
* after the call we are interposing returns.
* To call these functions and automatically preserve errno, use the internal_{fn} variants.
*/
#define GEN_FN_DEF_REAL(ret, name, ...) \
typedef ret (*fn_real_##name)(__VA_ARGS__); \
const fn_real_##name real_##name = (fn_real_##name)dlsym(RTLD_NEXT, #name);
#define GEN_FN_DEF_INTERNAL(ret, name, ...) \
template<typename ...TArgs> ret internal_##name(TArgs&& ...args) \
{ \
int prevErrno = errno; \
ret result = real_##name(std::forward<TArgs>(args)...); \
errno = prevErrno; \
return result; \
}
/*
* When interposing versioned symbols from glibc, dlsym will always pick up the oldest version by default.
* This is done for compatibility, but can cause issues if a binary is expecting a newer version of the function.
@ -57,10 +79,6 @@ static const char GLIBC_23[] = "GLIBC_2.3";
*
* To check what version of a libc function a binary is using, dump it with the following command: objdump -t </path/to/binary> | grep <function_name>
*/
#define GEN_FN_DEF_REAL(ret, name, ...) \
typedef ret (*fn_real_##name)(__VA_ARGS__); \
const fn_real_##name real_##name = (fn_real_##name)dlsym(RTLD_NEXT, #name);
#define GEN_FN_DEF_REAL_VERSIONED(version, ret, name, ...) \
typedef ret (*fn_real_##name)(__VA_ARGS__); \
const fn_real_##name real_##name = (fn_real_##name)dlvsym(RTLD_NEXT, #name, version);
@ -91,10 +109,12 @@ static const char GLIBC_23[] = "GLIBC_2.3";
// to retrieve the details in case of the failure.
#define GEN_FN_DEF(ret, name, ...) \
GEN_FN_DEF_REAL(ret, name, __VA_ARGS__) \
GEN_FN_DEF_INTERNAL(ret, name, __VA_ARGS__) \
GEN_FN_FWD(ret, name, __VA_ARGS__)
#define GEN_FN_DEF_VERSIONED(version, ret, name, ...) \
GEN_FN_DEF_REAL_VERSIONED(version, ret, name, __VA_ARGS__) \
GEN_FN_DEF_INTERNAL(ret, name, __VA_ARGS__) \
GEN_FN_FWD(ret, name, __VA_ARGS__)
#define GEN_FN_FWD(ret, name, ...) \
@ -435,31 +455,27 @@ public:
mode_t get_mode(const char *path)
{
int old = errno;
struct stat buf;
#if (__GLIBC__ == 2 && __GLIBC_MINOR__ < 33)
mode_t result = real___lxstat(1, path, &buf) == 0
mode_t result = internal___lxstat(1, path, &buf) == 0
#else
mode_t result = real_lstat(path, &buf) == 0
mode_t result = internal_lstat(path, &buf) == 0
#endif
? buf.st_mode
: 0;
errno = old;
return result;
}
mode_t get_mode(int fd)
{
int old = errno;
struct stat buf;
#if (__GLIBC__ == 2 && __GLIBC_MINOR__ < 33)
mode_t result = real___fxstat(1, fd, &buf) == 0
mode_t result = internal___fxstat(1, fd, &buf) == 0
#else
mode_t result = real_fstat(fd, &buf) == 0
mode_t result = internal_fstat(fd, &buf) == 0
#endif
? buf.st_mode
: 0;
errno = old;
return result;
}
@ -473,7 +489,7 @@ public:
{
char linkPath[100] = {0};
sprintf(linkPath, "/proc/%d/cwd", associatedPid);
if (real_readlink(linkPath, fullpath, size) == -1)
if (internal_readlink(linkPath, fullpath, size) == -1)
{
return NULL;
}