Skip to content

clone: Honor CLONE_PARENT #13255

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

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
Open

clone: Honor CLONE_PARENT #13255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions pkg/sentry/kernel/task_clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,13 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) {
if args.Flags&linux.CLONE_SIGHAND == 0 {
sh = sh.Fork()
}
tg = t.k.NewThreadGroup(pidns, sh, linux.Signal(args.ExitSignal), tg.limits.GetCopy())
termSig := linux.Signal(args.ExitSignal)
if args.Flags&linux.CLONE_PARENT != 0 {
t.tg.pidns.owner.mu.RLock()
termSig = t.tg.terminationSignal
t.tg.pidns.owner.mu.RUnlock()
}
tg = t.k.NewThreadGroup(pidns, sh, termSig, tg.limits.GetCopy())
tg.oomScoreAdj = atomicbitops.FromInt32(t.tg.oomScoreAdj.Load())
rseqAddr = t.rseqAddr
rseqSignature = t.rseqSignature
Expand Down Expand Up @@ -286,7 +292,7 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) {
SessionKeyring: sessionKeyring,
Origin: t.Origin,
}
if args.Flags&linux.CLONE_THREAD == 0 {
if args.Flags&(linux.CLONE_THREAD|linux.CLONE_PARENT) == 0 {
cfg.Parent = t
} else {
cfg.InheritParent = t
Expand Down
3 changes: 3 additions & 0 deletions pkg/sentry/syscalls/linux/sys_thread.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ func Clone3(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr,
if cloneArgs.Flags&linux.CLONE_DETACHED != 0 {
return 0, nil, linuxerr.EINVAL
}
if cloneArgs.Flags&(linux.CLONE_THREAD|linux.CLONE_PARENT) != 0 && cloneArgs.ExitSignal != 0 {
return 0, nil, linuxerr.EINVAL
}

ntid, ctrl, err := t.Clone(&cloneArgs)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions test/syscalls/linux/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ cc_binary(
malloc = "//test/util:errno_safe_allocator",
deps = select_gtest() + [
"//test/util:capability_util",
"//test/util:file_descriptor",
"//test/util:logging",
"//test/util:memory_util",
"//test/util:posix_error",
Expand Down
14 changes: 7 additions & 7 deletions test/syscalls/linux/cgroup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ TEST_F(Cgroup2Test, PidsEnforcement) {
constexpr int kCantFork = 2;
pid_t pid = fork();
if (pid == 0) {
wfd.reset();
close(wfd.release());
char token;
if (read(rfd.get(), &token, 1) <= 0) {
_exit(1);
Expand Down Expand Up @@ -1490,7 +1490,7 @@ TEST_F(Cgroup2Test, InotifyEventsOnExit) {

pid_t pid = fork();
if (pid == 0) {
wfd.reset();
close(wfd.release());
char token;
if (read(rfd.get(), &token, 1) <= 0) {
_exit(1);
Expand Down Expand Up @@ -1528,7 +1528,7 @@ TEST_F(Cgroup2Test, ZombieCgroupMembership) {

pid_t pid = fork();
if (pid == 0) {
wfd.reset();
close(wfd.release());
char token;
if (read(rfd.get(), &token, 1) <= 0) {
_exit(1);
Expand Down Expand Up @@ -1831,7 +1831,7 @@ TEST_F(Cgroup2Test, KillTree) {

pid_t pid1 = fork();
if (pid1 == 0) {
wfd.reset();
close(wfd.release());
char token;
if (read(rfd.get(), &token, 1) <= 0) {
_exit(1);
Expand All @@ -1842,7 +1842,7 @@ TEST_F(Cgroup2Test, KillTree) {

pid_t pid2 = fork();
if (pid2 == 0) {
wfd.reset();
close(wfd.release());
char token;
if (read(rfd.get(), &token, 1) <= 0) {
_exit(1);
Expand Down Expand Up @@ -1969,8 +1969,8 @@ TEST_F(Cgroup2Test, ThreadedDomainComplexTopology) {

pid_t p2 = fork();
if (p2 == 0) {
exit_wfd.reset();
start_rfd.reset();
close(exit_wfd.release());
close(start_rfd.release());
// Move the child leader into child2.
ASSERT_NO_ERRNO(child2.WriteIntegerControlFile("cgroup.threads", gettid()));

Expand Down
6 changes: 3 additions & 3 deletions test/syscalls/linux/exec.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ void writeAndWaitForPid(int child_pid, int pipe_fd) {
TEST_PCHECK_MSG(waitpid(child_pid, &status, 0) == child_pid,
"waitpid failed.");
TEST_CHECK(WIFEXITED(status) && WEXITSTATUS(status) == 0);
exit(42);
_exit(42);
}

void ExecWithThread() {
Expand All @@ -1266,7 +1266,7 @@ void ExecWithThread() {
const ExecveArray envv;

execve("/proc/self/exe", argv.get(), envv.get());
exit(errno);
_exit(errno);
}

void ExecFromThread() {
Expand All @@ -1275,7 +1275,7 @@ void ExecFromThread() {
const ExecveArray envv;

execve("/proc/self/exe", argv.get(), envv.get());
exit(errno);
_exit(errno);
});

while (true) {
Expand Down
6 changes: 3 additions & 3 deletions test/syscalls/linux/exit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ TEST(ExitTest, CloseFds) {

pid_t pid = fork();
if (pid == 0) {
read_fd.reset();
close(read_fd.release());

SleepSafe(absl::Seconds(10));

Expand Down Expand Up @@ -109,7 +109,7 @@ TEST(ExitTest, SigkillZombieGroup) {

pid_t pid = fork();
if (pid == 0) {
read_fd.reset();
close(read_fd.release());

_exit(0);
}
Expand Down Expand Up @@ -140,7 +140,7 @@ TEST(ExitTest, SigkillZombieThread) {

pid_t pid = fork();
if (pid == 0) {
read_fd.reset();
close(read_fd.release());

syscall(SYS_exit, 0);
}
Expand Down
78 changes: 76 additions & 2 deletions test/syscalls/linux/fork.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "gtest/gtest.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "test/util/file_descriptor.h"
#include "test/util/linux_capability_util.h"
#include "test/util/logging.h"
#include "test/util/memory_util.h"
Expand Down Expand Up @@ -515,6 +516,79 @@ int clone3(struct clone_args* ca, size_t size) {
return syscall(SYS_clone3, ca, size);
}

TEST(CloneTest, CloneParent) {
// A is the test process. B is the child. C is the grandchild.
// C is created with CLONE_PARENT, so it should be a adopted by A.
pid_t a_pid = getpid();

int fds[2];
ASSERT_THAT(pipe(fds), SyscallSucceeds());
FileDescriptor rfd(fds[0]);
FileDescriptor wfd(fds[1]);

pid_t b_pid = fork();
if (b_pid == 0) {
if (close(rfd.release()) != 0) {
_exit(1);
}

clone_args ca = {};
ca.flags = CLONE_PARENT;
ca.exit_signal = 0; // clone3 will fail without this.
pid_t c_pid = clone3(&ca, sizeof(ca));
if (c_pid == 0) {
if (getppid() != a_pid) {
_exit(42); // CLONE_PARENT failed.
}
_exit(0);
}
if (c_pid < 0) {
_exit(1); // C could not be created.
}
if (write(wfd.get(), &c_pid, sizeof(c_pid)) != sizeof(c_pid)) {
_exit(1);
}

// B should fail to wait on C with ECHILD if CLONE_PARENT worked.
int status = -1;
int rval = wait4(c_pid, &status, __WALL, NULL);
if (rval != -1 || errno != ECHILD) {
_exit(43);
}
_exit(0);
}
ASSERT_THAT(b_pid, SyscallSucceeds());
wfd.reset();

// A's wait on B.
int status;
EXPECT_THAT(waitpid(b_pid, &status, 0), SyscallSucceedsWithValue(b_pid));
ASSERT_TRUE(WIFEXITED(status));
EXPECT_EQ(WEXITSTATUS(status), 0);

// A's wait on C.
pid_t c_pid = -1;
EXPECT_THAT(read(rfd.get(), &c_pid, sizeof(c_pid)),
SyscallSucceedsWithValue(sizeof(c_pid)));
ASSERT_GT(c_pid, 0);
EXPECT_THAT(wait4(c_pid, &status, __WALL, NULL),
SyscallSucceedsWithValue(c_pid));
ASSERT_TRUE(WIFEXITED(status));
EXPECT_EQ(WEXITSTATUS(status), 0);
}

TEST(CloneTest, CloneParentOrThreadWithExitSignalFails) {
clone_args ca = {};
ca.flags = CLONE_PARENT;
ca.exit_signal = SIGCHLD;
EXPECT_THAT(clone3(&ca, sizeof(ca)), SyscallFailsWithErrno(EINVAL));

ca = {};
ca.flags = CLONE_THREAD;
ca.exit_signal = SIGCHLD;
EXPECT_THAT(clone3(&ca, sizeof(ca)), SyscallFailsWithErrno(EINVAL));
}

// Checks that clone fails for any unsupported flag.
TEST(CloneTest, Clone3UnknownFlag) {
clone_args ca = {};
Expand All @@ -532,7 +606,7 @@ TEST(CloneTest, Clone3AsClone) {
EXPECT_THAT(child_pid = clone3(&ca, sizeof(ca)), SyscallSucceeds());

if (child_pid == 0) {
exit(0);
_exit(0);
}

int status;
Expand All @@ -555,7 +629,7 @@ TEST(CloneTest, Clone3Basic) {
EXPECT_EQ(store_child_tid, child_pid);

if (child_pid == 0) {
exit(0);
_exit(0);
}

int status;
Expand Down
4 changes: 2 additions & 2 deletions test/syscalls/linux/kill.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ TEST(KillTest, CanKillAllPIDs) {

pid_t pid = fork();
if (pid == 0) {
read_fd.reset();
close(read_fd.release());

struct sigaction sa;
sa.sa_sigaction = SigHandler;
Expand All @@ -81,7 +81,7 @@ TEST(KillTest, CanKillAllPIDs) {
MaybeSave();

// Indicate to the parent that we're ready.
write_fd.reset();
close(write_fd.release());

// Wait until we get the signal from the parent.
while (true) {
Expand Down
2 changes: 1 addition & 1 deletion test/syscalls/linux/sigaltstack.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ TEST(SigaltstackTest, SetCurrentStack) {
// Should not be able to disable the stack.
stack.ss_flags = SS_DISABLE;
TEST_CHECK(sigaltstack(&stack, nullptr) == -1 && errno == EPERM);
exit(0);
_exit(0);
},
::testing::ExitedWithCode(0), "");
}
Expand Down
2 changes: 1 addition & 1 deletion test/syscalls/linux/sigtimedwait.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ TEST(SigtimedwaitTest, SIGKILLUncaught) {

pid_t pid = fork();
if (pid == 0) {
rfd.reset();
close(rfd.release());

sigset_t mask;
sigemptyset(&mask);
Expand Down
2 changes: 1 addition & 1 deletion test/syscalls/linux/sync_file_range.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ TEST(SyncFileRangeTest, CannotSyncFileRangeOnUnopenedFd) {

pid_t pid = fork();
if (pid == 0) {
f.reset();
close(f.release());

// fd is now invalid.
TEST_CHECK(sync_file_range(fd, 0, 0, SYNC_FILE_RANGE_WRITE) == -1);
Expand Down
4 changes: 2 additions & 2 deletions test/syscalls/linux/wait.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,12 +876,12 @@ TEST(WaitTest, TraceeWALL) {
pid_t child = fork();
if (child == 0) {
// Child.
rfd.reset();
close(rfd.release());

TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, nullptr, nullptr) == 0);

// Notify parent that we're now a tracee.
wfd.reset();
close(wfd.release());

_exit(0);
}
Expand Down
Loading