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.

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

Conversation

MasloMaslane
Copy link
Member

Closes #39

@MasloMaslane MasloMaslane self-assigned this Nov 27, 2023
@MasloMaslane MasloMaslane changed the title Add clone3 and statx sys calls Add clone3 and statx syscalls Nov 27, 2023
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

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

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:

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.

@MasloMaslane
Copy link
Member Author

I also added fstatat64, because tests were failing. From what I read it's safe to add

@MasloMaslane
Copy link
Member Author

We can't merge this, the tests are currently failing

@Wolf480pl
Copy link
Member

oh, that's weird

@Wolf480pl
Copy link
Member

ok, looks like it failed when trying to trace the cloned thread, see comment in diff

@teqwve teqwve mentioned this pull request Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Intercepted forbidden syscall' when running check
2 participants