Skip to content

Commit

Permalink
Linux sandbox: handle faccessat2 syscall
Browse files Browse the repository at this point in the history
Starting with Linux v5.8 a new syscall named faccessat2 was
added to correctly implement the POSIX specified faccessat
by properly handling new a fourth "flags" argument which was
missing in the original faccessat in turn causing bugs in
glibc which tried to "emulate" it.

Starting with glibc 2.33, the new faccessat2 syscall is used
so we need to handle it in the sandbox.

BUG=b:187795855

Change-Id: I98117e6726a58b3a60a8b6490afc6a749b8b27b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3114287
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Commit-Queue: Adrian Ratiu <adrian.ratiu@collabora.corp-partner.google.com>
Auto-Submit: Adrian Ratiu <adrian.ratiu@collabora.corp-partner.google.com>
Cr-Commit-Position: refs/heads/main@{#916258}
  • Loading branch information
Adrian Ratiu authored and Chromium LUCI CQ committed Aug 28, 2021
1 parent 60e827f commit 909ea95
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ intptr_t BrokerOpenTrapHandler(const struct arch_seccomp_data& args,
BrokerProcess* broker_process = static_cast<BrokerProcess*>(aux);
switch (args.nr) {
case __NR_faccessat: // access is a wrapper of faccessat in android
case __NR_faccessat2:
BPF_ASSERT(static_cast<int>(args.args[0]) == AT_FDCWD);
return broker_process->GetBrokerClientSignalBased()->Access(
reinterpret_cast<const char*>(args.args[1]),
Expand Down Expand Up @@ -122,6 +123,7 @@ class DenyOpenPolicy : public bpf_dsl::Policy {

switch (sysno) {
case __NR_faccessat:
case __NR_faccessat2:
#if defined(__NR_access)
case __NR_access:
#endif
Expand Down
1 change: 1 addition & 0 deletions sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ bool SyscallSets::IsFileSystem(int sysno) {

case __NR_execve:
case __NR_faccessat: // EPERM not a valid errno.
case __NR_faccessat2:
case __NR_fchmodat:
case __NR_fchownat: // Should be called chownat ?
#if defined(__x86_64__) || defined(__aarch64__)
Expand Down
1 change: 1 addition & 0 deletions sandbox/linux/syscall_broker/broker_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ bool BrokerProcess::IsSyscallBrokerable(int sysno, bool fast_check) const {
case __NR_access:
#endif
case __NR_faccessat:
case __NR_faccessat2:
return !fast_check || allowed_command_set_.test(COMMAND_ACCESS);

#if !defined(__aarch64__) && !defined(OS_ANDROID)
Expand Down
1 change: 1 addition & 0 deletions sandbox/linux/syscall_broker/broker_process_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,7 @@ TEST(BrokerProcess, IsSyscallAllowed) {
const base::flat_map<BrokerCommand, base::flat_set<int>> kSysnosForCommand = {
{COMMAND_ACCESS,
{__NR_faccessat,
__NR_faccessat2,
#if defined(__NR_access) && !defined(OS_ANDROID)
__NR_access
#endif
Expand Down
5 changes: 5 additions & 0 deletions sandbox/linux/syscall_broker/syscall_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ int SyscallDispatcher::DispatchSyscall(const arch_seccomp_data& args) {
#endif
#if defined(__NR_faccessat)
case __NR_faccessat:
#endif
#if defined(__NR_faccessat2)
case __NR_faccessat2:
#endif
#if defined(__NR_faccessat) || defined(__NR_faccessat2)
if (static_cast<int>(args.args[0]) != AT_FDCWD)
return -EPERM;
return Access(reinterpret_cast<const char*>(args.args[1]),
Expand Down
5 changes: 5 additions & 0 deletions sandbox/policy/linux/bpf_broker_policy_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ ResultExpr BrokerProcessPolicy::EvaluateSyscall(int sysno) const {
#endif
#if defined(__NR_faccessat)
case __NR_faccessat:
#endif
#if defined(__NR_faccessat2)
case __NR_faccessat2:
#endif
#if defined(__NR_faccessat) || defined(__NR_faccessat2)
if (allowed_command_set_.test(syscall_broker::COMMAND_ACCESS))
return Allow();
break;
Expand Down

0 comments on commit 909ea95

Please sign in to comment.