Skip to content

Commit 790f0e1

Browse files
AkihiroSudaAkihiroSuda
authored
Merge pull request #514 from fahedouch/fix-Pdeathsig-behavior
child: refactor command execution to use goroutines with Pdeathsig
2 parents e83d763 + 2ca0537 commit 790f0e1

File tree

3 files changed

+177
-13
lines changed

3 files changed

+177
-13
lines changed

‎.github/workflows/main.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ jobs:
5252
run: docker run --rm --privileged --sysctl net.ipv6.conf.all.disable_ipv6=0 rootlesskit:test-integration ./integration-ipv6.sh
5353
- name: "Integration test: systemd socket activation"
5454
run: docker run --rm --net=none --privileged rootlesskit:test-integration ./integration-systemd-socket.sh
55+
- name: "Integration test: pdeathsig"
56+
run: docker run --rm --privileged rootlesskit:test-integration ./integration-pdeathsig.sh
5557
- name: "Integration test: Network (network driver=slirp4netns)"
5658
run: |
5759
docker run --rm --privileged rootlesskit:test-integration ./integration-net.sh slirp4netns

‎hack/integration-pdeathsig.sh‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
#!/bin/bash
2+
# Test script to verify Pdeathsig behavior using rootlesskit itself
3+
# This script:
4+
# 1. Uses rootlesskit to spawn a long-running process
5+
# 2. Kills the rootlesskit parent process
6+
# 3. Verifies that the child process is killed as expected
7+
# 4. Tests both with --reaper true and --reaper false
8+
9+
source $(realpath $(dirname $0))/common.inc.sh
10+
11+
INFO "Starting Pdeathsig test using rootlesskit..."
12+
13+
# Function to run the test with a specific reaper setting
14+
run_test() {
15+
local reaper_setting=$1
16+
INFO "Testing with --reaper $reaper_setting"
17+
18+
# Create a temporary directory for test artifacts
19+
TEMP_DIR=$(mktemp -d)
20+
INFO "Created temporary directory: $TEMP_DIR"
21+
22+
# Create a marker file that will be touched by the child process if it's still alive
23+
MARKER_FILE="$TEMP_DIR/child_still_alive"
24+
25+
# Create a script that will be executed by rootlesskit
26+
CHILD_SCRIPT="$TEMP_DIR/child_script.sh"
27+
cat > "$CHILD_SCRIPT" << 'EOF'
28+
#!/bin/bash
29+
echo "Child process started with PID: $$"
30+
echo "Parent PID: $PPID"
31+
32+
# Register a trap to handle signals
33+
trap 'echo "Child received signal, exiting"; exit 1' TERM INT
34+
35+
# Run for 30 seconds, checking if parent is still alive
36+
for i in {1..30}; do
37+
echo "Child still running (iteration $i)..."
38+
39+
# Check if parent has changed (died)
40+
CURRENT_PPID=$(ps -o ppid= -p $$)
41+
if [ "$CURRENT_PPID" != "$PPID" ]; then
42+
echo "Parent changed from $PPID to $CURRENT_PPID"
43+
if [ "$CURRENT_PPID" = "1" ]; then
44+
echo "Parent is now init (PID 1), parent has died"
45+
echo "Child should be killed by Pdeathsig, but if you see this message, it wasn't"
46+
touch MARKER_FILE_PLACEHOLDER
47+
exit 1
48+
fi
49+
fi
50+
51+
sleep 1
52+
done
53+
54+
# If we reach here, the child wasn't killed
55+
echo "Child completed normally (this shouldn't happen if Pdeathsig is working)"
56+
touch MARKER_FILE_PLACEHOLDER
57+
EOF
58+
59+
# Replace the placeholder with the actual marker file path
60+
sed -i "s|MARKER_FILE_PLACEHOLDER|$MARKER_FILE|g" "$CHILD_SCRIPT"
61+
chmod +x "$CHILD_SCRIPT"
62+
63+
# Start rootlesskit with the child script
64+
INFO "Starting rootlesskit with --reaper $reaper_setting..."
65+
if [ "$reaper_setting" = "true" ]; then
66+
$ROOTLESSKIT --reaper $reaper_setting --pidns "$CHILD_SCRIPT" &
67+
else
68+
$ROOTLESSKIT --reaper $reaper_setting "$CHILD_SCRIPT" &
69+
fi
70+
ROOTLESSKIT_PID=$!
71+
INFO "Rootlesskit started with PID: $ROOTLESSKIT_PID"
72+
73+
# Wait a moment for the child to start
74+
sleep 2
75+
76+
# Find the child process
77+
ROOTLESSKIT_CHILD_PID=$(pgrep -P $ROOTLESSKIT_PID)
78+
if [ -z "$ROOTLESSKIT_CHILD_PID" ]; then
79+
ERROR "Failed to find rootlesskit child process"
80+
return 1
81+
fi
82+
INFO "Found rootlesskit child process with PID: $ROOTLESSKIT_CHILD_PID"
83+
84+
# Kill the rootlesskit process
85+
INFO "Killing rootlesskit process (PID: $ROOTLESSKIT_PID)..."
86+
kill -9 $ROOTLESSKIT_PID
87+
88+
# Wait a moment for the rootlesskit child to be killed
89+
sleep 2
90+
91+
# Check if the rootlesskit child process is still running
92+
if ps -p $ROOTLESSKIT_CHILD_PID > /dev/null; then
93+
ERROR "FAIL: Rootlesskit Child process (PID: $ROOTLESSKIT_CHILD_PID) is still running after rootlesskit parent was killed"
94+
kill -9 $ROOTLESSKIT_CHILD_PID # Clean up
95+
return 1
96+
else
97+
INFO "PASS: Rootlesskit Child process (PID: $ROOTLESSKIT_CHILD_PID) was killed as expected"
98+
fi
99+
100+
# Check if the marker file exists
101+
if [ -f "$MARKER_FILE" ]; then
102+
ERROR "FAIL: Marker file exists, which means the child process wasn't killed by Pdeathsig"
103+
return 1
104+
else
105+
INFO "PASS: Marker file doesn't exist, which means the child process was killed by Pdeathsig"
106+
fi
107+
108+
INFO "Test with --reaper $reaper_setting completed successfully!"
109+
rm -rf "$TEMP_DIR"
110+
return 0
111+
}
112+
113+
# Run tests with both reaper settings
114+
if ! run_test "true"; then
115+
ERROR "Test with --reaper true failed"
116+
exit 1
117+
fi
118+
119+
if ! run_test "false"; then
120+
ERROR "Test with --reaper false failed"
121+
exit 1
122+
fi
123+
124+
INFO "All tests completed successfully!"
125+
exit 0

‎pkg/child/child.go‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,6 @@ func Child(opt Opt) error {
442442

443443
// The parent calls child with Pdeathsig, but it is cleared when newuidmap SUID binary is called
444444
// https://github.com/rootless-containers/rootlesskit/issues/65#issuecomment-492343646
445-
runtime.LockOSThread()
446-
err = unix.Prctl(unix.PR_SET_PDEATHSIG, uintptr(unix.SIGKILL), 0, 0, 0)
447-
runtime.UnlockOSThread()
448-
if err != nil {
449-
return err
450-
}
451445
os.Unsetenv(opt.PipeFDEnvKey)
452446
if err := pipeR.Close(); err != nil {
453447
return fmt.Errorf("failed to close fd %d: %w", pipeFD, err)
@@ -483,17 +477,50 @@ func Child(opt Opt) error {
483477
if err != nil {
484478
return err
485479
}
480+
481+
// Create a channel to receive errors from the goroutine
482+
cmdErrCh := make(chan error, 1)
483+
486484
if opt.Reaper {
487-
if err := runAndReap(cmd); err != nil {
485+
// Launch a goroutine to execute the command with Pdeathsig
486+
go func() {
487+
// Lock the goroutine to the OS thread
488+
runtime.LockOSThread()
489+
defer runtime.UnlockOSThread()
490+
491+
// Set the parent death signal
492+
if err := unix.Prctl(unix.PR_SET_PDEATHSIG, uintptr(unix.SIGKILL), 0, 0, 0); err != nil {
493+
cmdErrCh <- err
494+
return
495+
}
496+
497+
// Run the command
498+
cmdErrCh <- runAndReap(cmd)
499+
}()
500+
501+
// Wait for the command to complete
502+
if err := <-cmdErrCh; err != nil {
488503
return fmt.Errorf("command %v exited: %w", opt.TargetCmd, err)
489504
}
490505
} else {
491-
if err := cmd.Start(); err != nil {
492-
return fmt.Errorf("command %v exited: %w", opt.TargetCmd, err)
493-
}
494-
sigc := sigproxy.ForwardAllSignals(context.TODO(), cmd.Process.Pid)
495-
defer sigproxysignal.StopCatch(sigc)
496-
if err := cmd.Wait(); err != nil {
506+
// Launch a goroutine to execute the command with Pdeathsig
507+
go func() {
508+
// Lock the goroutine to the OS thread
509+
runtime.LockOSThread()
510+
defer runtime.UnlockOSThread()
511+
512+
// Set the parent death signal
513+
if err := unix.Prctl(unix.PR_SET_PDEATHSIG, uintptr(unix.SIGKILL), 0, 0, 0); err != nil {
514+
cmdErrCh <- err
515+
return
516+
}
517+
518+
// Run the command without reaping
519+
cmdErrCh <- runWithoutReap(cmd)
520+
}()
521+
522+
// Wait for the command to complete
523+
if err := <-cmdErrCh; err != nil {
497524
return fmt.Errorf("command %v exited: %w", opt.TargetCmd, err)
498525
}
499526
}
@@ -514,6 +541,16 @@ func setMountPropagation(propagation string) error {
514541
return nil
515542
}
516543

544+
func runWithoutReap(cmd *exec.Cmd) error {
545+
cmd.SysProcAttr.Setsid = true
546+
if err := cmd.Start(); err != nil {
547+
return err
548+
}
549+
sigc := sigproxy.ForwardAllSignals(context.TODO(), cmd.Process.Pid)
550+
defer sigproxysignal.StopCatch(sigc)
551+
return cmd.Wait()
552+
}
553+
517554
func runAndReap(cmd *exec.Cmd) error {
518555
c := make(chan os.Signal, 32)
519556
signal.Notify(c, syscall.SIGCHLD)

0 commit comments

Comments
 (0)