Skip to content

Commit f7b2779

Browse files
kolyshkingopherbot
authored andcommitted
syscall: fix getting pidfd when using CLONE_NEWUSER
While working on CL 528798, I found out that sys.PidFD field (added in CL 520266) is not filled in when CLONE_NEWUSER is used. This happens because the code assumed that the parent and the child run in the same memory space. This assumption is right only when CLONE_VM is used for clone syscall, and the code only sets CLONE_VM when CLONE_NEWUSER is not used. Fix this, and add a test case (which fails before the fix). Updates #51246. Change-Id: I805203c1369cadd63d769568b132a9ffd92cc184 Reviewed-on: https://go-review.googlesource.com/c/go/+/542698 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Auto-Submit: Michael Pratt <[email protected]>
1 parent cfb2817 commit f7b2779

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

src/syscall/exec_linux.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func runtime_AfterForkInChild()
133133
func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
134134
// Set up and fork. This returns immediately in the parent or
135135
// if there's an error.
136-
upid, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
136+
upid, pidfd, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
137137
if locked {
138138
runtime_AfterFork()
139139
}
@@ -143,6 +143,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
143143

144144
// parent; return PID
145145
pid = int(upid)
146+
if sys.PidFD != nil {
147+
*sys.PidFD = int(pidfd)
148+
}
146149

147150
if sys.UidMappings != nil || sys.GidMappings != nil {
148151
Close(mapPipe[0])
@@ -210,7 +213,7 @@ type cloneArgs struct {
210213
//go:noinline
211214
//go:norace
212215
//go:nocheckptr
213-
func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, err1 Errno, mapPipe [2]int, locked bool) {
216+
func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, pidfd int32, err1 Errno, mapPipe [2]int, locked bool) {
214217
// Defined in linux/prctl.h starting with Linux 4.3.
215218
const (
216219
PR_CAP_AMBIENT = 0x2f
@@ -241,12 +244,12 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
241244
uidmap, setgroups, gidmap []byte
242245
clone3 *cloneArgs
243246
pgrp int32
244-
pidfd _C_int = -1
245247
dirfd int
246248
cred *Credential
247249
ngroups, groups uintptr
248250
c uintptr
249251
)
252+
pidfd = -1
250253

251254
rlim := origRlimitNofile.Load()
252255

@@ -341,10 +344,6 @@ func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, att
341344

342345
// Fork succeeded, now in child.
343346

344-
if sys.PidFD != nil {
345-
*sys.PidFD = int(pidfd)
346-
}
347-
348347
// Enable the "keep capabilities" flag to set ambient capabilities later.
349348
if len(sys.AmbientCaps) > 0 {
350349
_, _, err1 = RawSyscall6(SYS_PRCTL, PR_SET_KEEPCAPS, 1, 0, 0, 0, 0)

src/syscall/exec_linux_test.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ func TestCloneTimeNamespace(t *testing.T) {
522522
}
523523
}
524524

525-
func testPidFD(t *testing.T) error {
525+
func testPidFD(t *testing.T, userns bool) error {
526526
testenv.MustHaveExec(t)
527527

528528
if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
@@ -541,6 +541,9 @@ func testPidFD(t *testing.T) error {
541541
cmd.SysProcAttr = &syscall.SysProcAttr{
542542
PidFD: &pidfd,
543543
}
544+
if userns {
545+
cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWUSER
546+
}
544547
if err := cmd.Start(); err != nil {
545548
return err
546549
}
@@ -572,7 +575,13 @@ func testPidFD(t *testing.T) error {
572575
}
573576

574577
func TestPidFD(t *testing.T) {
575-
if err := testPidFD(t); err != nil {
578+
if err := testPidFD(t, false); err != nil {
579+
t.Fatal("can't start a process:", err)
580+
}
581+
}
582+
583+
func TestPidFDWithUserNS(t *testing.T) {
584+
if err := testPidFD(t, true); err != nil {
576585
t.Fatal("can't start a process:", err)
577586
}
578587
}
@@ -581,7 +590,7 @@ func TestPidFDClone3(t *testing.T) {
581590
*syscall.ForceClone3 = true
582591
defer func() { *syscall.ForceClone3 = false }()
583592

584-
if err := testPidFD(t); err != nil {
593+
if err := testPidFD(t, false); err != nil {
585594
if testenv.SyscallIsNotSupported(err) {
586595
t.Skip("clone3 not supported:", err)
587596
}

0 commit comments

Comments
 (0)