Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clone3 and statx syscalls #40

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add clone3 and statx syscalls #40

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
78cd120
Add clone syscall
MasloMaslane Nov 27, 2023
71b2745
Add statx syscall
MasloMaslane Nov 27, 2023
57370da
Fix typo
MasloMaslane Nov 27, 2023
6b99854
Block clone3 with ENOSYS
MasloMaslane Nov 29, 2023
dcb2d92
Add fstatat64
MasloMaslane Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/limits/ThreadsLimitListener.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ ThreadsLimitListener::ThreadsLimitListener(int32_t threadsLimit)
// Disable threads support
syscallRules_.emplace_back(
seccomp::SeccompRule("clone", seccomp::action::ActionKill{}));
syscallRules_.emplace_back(
seccomp::SeccompRule("clone3", seccomp::action::ActionKill{}));
}
else {
// Enable threads support
Expand All @@ -33,10 +35,19 @@ ThreadsLimitListener::ThreadsLimitListener(int32_t threadsLimit)
"clone",
seccomp::action::ActionAllow{},
(Arg(2) & CLONE_VM) == CLONE_VM));
syscallRules_.emplace_back(seccomp::SeccompRule(
"clone3",
seccomp::action::ActionAllow{},
(Arg(2) & CLONE_VM) == CLONE_VM));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhh... just checked what the args of clone3 are, and

long syscall(SYS_clone3, struct clone_args *cl_args, size_t size);

which translates to

clone3(struct clone_args *cl_args, size_t size);

Arg(0) is a pointer to a struct with all the stuff that'd normally go into clone args
Arg(1) is the size of that struct
Arg(2) is not a thing

So our check for CLONE_VM flag won't work correctly.

I don't think we can support clone3. Unless the kernel and libseccomp have some nice way of getting at the parameters in the struct, I think we should just ENOSYS this one too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added ENOSYS for clone3, but now this test is failing:

1: ======================================================================
1: ERROR: test_1_sec_program_threads_1 (testsuits.test_times.TestReportedTimes)
1: ----------------------------------------------------------------------
1: Traceback (most recent call last):
1:   File "/sio2jail/test/testsuits/test_times.py", line 24, in test_1_sec_program_threads_1
1:     self.assertAlmostEqual(result.time, 1.0)
1:   File "/usr/local/lib/python3.9/unittest/case.py", line 868, in assertAlmostEqual
1:     diff = abs(first - second)
1: TypeError: unsupported operand type(s) for -: 'NoneType' and 'float'
1: -------------------- >> begin captured stdout << ---------------------
1: running:
1: /sio2jail/build/./src/sio2jail -b /sio2jail/build/./boxes/./minimal/:/:ro -t 1 -m 1G /sio2jail/build/./test/src/1-sec-prog-th flat 1
1:
1: result: 2
1:
1: stdout:
1:
1: stderr:
1: Exception occurred: System error occured: ptrace setopts child failed: No such process: error 3: No such process
1:
1:
1: --------------------- >> end captured stdout << ----------------------
1:
1: ----------------------------------------------------------------------
1: Ran 14 tests in 1.575s
1:
1: FAILED (errors=1)
1/1 Test #1: python ...........................***Failed    1.75 sec

When clone3 is allowed, this test passes

Copy link
Member

@Wolf480pl Wolf480pl Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I think I know why this happens.

TraceExecutor gets clone events here

sio2jail/src/tracer/TraceExecutor.cc

Lines 94 to 95 in 8e65e31

if (executeEvent.signal == (SIGTRAP | (PTRACE_EVENT_CLONE << 8))) {
action = std::max(action, onEventClone(event, tracee));

not due to seccomp traps, but due to PTRACE_O_TRACECLONE.

It then tries to get child's pid, and set up ptrace of the child too:

sio2jail/src/tracer/TraceExecutor.cc

Lines 137 to 156 in 8e65e31

TraceAction TraceExecutor::onEventClone(
const TraceEvent& event,
Tracee& tracee) {
pid_t traceeChildPid{-1};
withErrnoCheck(
"ptrace get child pid",
ptrace,
PTRACE_GETEVENTMSG,
event.executeEvent.pid,
nullptr,
&traceeChildPid);
logger::debug(VAR(event.executeEvent.pid), VAR(traceeChildPid));
withErrnoCheck(
"ptrace setopts child",
ptrace,
PTRACE_SETOPTIONS,
traceeChildPid,
nullptr,
PTRACE_OPTIONS);

I suspect this triggers even if the a clone (in this case clone3) fails, and TraceExecutor doesn't check for that.

Maybe ptrace has some way to get the clone's error code, so that we can check if it succeeded.
If there's no way to do that, I guess we can just catch the ESRCH returned by PTRACE_SETOPTIONS, assume clone failed and move on, so that the process can retry with the old clone instead of clone3.


syscallRules_.emplace_back(seccomp::SeccompRule(
"clone",
seccomp::action::ActionKill(),
(Arg(2) & CLONE_VM) == 0));
syscallRules_.emplace_back(seccomp::SeccompRule(
"clone3",
seccomp::action::ActionKill(),
(Arg(2) & CLONE_VM) == 0));

// And various thread related
syscallRules_.emplace_back(seccomp::SeccompRule(
Expand Down
1 change: 1 addition & 0 deletions src/seccomp/policy/DefaultPolicy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ void DefaultPolicy::addFileSystemAccessRules(bool readOnly) {
"statfs64",
"fstatfs",
"fstatfs64",
"statx",
}) {
rules_.emplace_back(SeccompRule(syscall, action::ActionErrno(ENOSYS)));
}
Expand Down