Commit c8c93ef

committed

syscall: exec_linux: switch to F_DUPFD_CLOEXEC in clobber-prevention logic

The existing clobber-prevention logic can end up clobbering random file descriptors, which can cause issues on Linux if a user wants to execute a /proc/self/fd/$n handle that isn't included in attr.Files. Similar logic already exists for the BSDs and Solaris. In addition, the F_DUPFD_CLOEXEC makes the clobber-prevention logic much simpler to follow. Closes #61751

1 parent 5d53050 commit c8c93efCopy full SHA for c8c93ef

File tree

1 file changed

+15

-25

lines changed

src/syscall
- exec_linux.go

1 file changed

+15

-25

lines changed

`‎src/syscall/exec_linux.go`

Lines changed: 15 additions & 25 deletions

Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,6 @@ func forkAndExecInChild1(argv0 byte, argv, envv []byte, chroot, dir *byte, att`
`227`	`227`	`// by an otherwise-correct change in the compiler.`
`228`	`228`	`var (`
`229`	`229`	`err2 Errno`
`230`		`- nextfd int`
`231`	`230`	`i int`
`232`	`231`	`caps caps`
`233`	`232`	`fd1, flags uintptr`
`@@ -263,18 +262,11 @@ func forkAndExecInChild1(argv0 byte, argv, envv []byte, chroot, dir *byte, att`
`263`	`262`	`// Record parent PID so child can test if it has died.`
`264`	`263`	`ppid, _ := rawSyscallNoError(SYS_GETPID, 0, 0, 0)`
`265`	`264`
`266`		`- // Guard against side effects of shuffling fds below.`
`267`		`- // Make sure that nextfd is beyond any currently open files so`
`268`		`- // that we can't run the risk of overwriting any of them.`
	`265`	`+ // Used to guard against side effects of shuffling fds below.`
`269`	`266`	`fd := make([]int, len(attr.Files))`
`270`		`- nextfd = len(attr.Files)`
`271`	`267`	`for i, ufd := range attr.Files {`
`272`		`- if nextfd < int(ufd) {`
`273`		`- nextfd = int(ufd)`
`274`		`- }`
`275`	`268`	`fd[i] = int(ufd)`
`276`	`269`	`}`
`277`		`- nextfd++`
`278`	`270`
`279`	`271`	`// Allocate another pipe for parent to child communication for`
`280`	`272`	`// synchronizing writing of User ID/Group ID mappings.`
`@@ -541,28 +533,26 @@ func forkAndExecInChild1(argv0 byte, argv, envv []byte, chroot, dir *byte, att`
`541`	`533`	`}`
`542`	`534`	`}`
`543`	`535`
`544`		`- // Pass 1: look for fd[i] < i and move those up above len(fd)`
`545`		`- // so that pass 2 won't stomp on an fd it needs later.`
`546`		`- if pipe < nextfd {`
`547`		`- _, _, err1 = RawSyscall(SYS_DUP3, uintptr(pipe), uintptr(nextfd), O_CLOEXEC)`
`548`		`- if err1 != 0 {`
`549`		`- goto childerror`
`550`		`- }`
`551`		`- pipe = nextfd`
`552`		`- nextfd++`
`553`		`- }`
	`536`	`+ // Pass 1: duplicate any fd[i] < i as O_CLOEXEC to make sure we don't`
	`537`	`+ // clobber them when we shuffle the fds to match the attr.Files layout.`
`554`	`538`	`for i = 0; i < len(fd); i++ {`
`555`	`539`	`if fd[i] >= 0 && fd[i] < i {`
`556`		`- if nextfd == pipe { // don't stomp on pipe`
`557`		`- nextfd++`
`558`		`- }`
`559`		`- _, _, err1 = RawSyscall(SYS_DUP3, uintptr(fd[i]), uintptr(nextfd), O_CLOEXEC)`
	`540`	`+ fd1, _, err1 = RawSyscall(fcntl64Syscall, uintptr(fd[i]), F_DUPFD_CLOEXEC, 0)`
`560`	`541`	`if err1 != 0 {`
`561`	`542`	`goto childerror`
`562`	`543`	`}`
`563`		`- fd[i] = nextfd`
`564`		`- nextfd++`
	`544`	`+ fd[i] = int(fd1)`
	`545`	`+ }`
	`546`	`+ }`
	`547`	`+`
	`548`	`+ // Guard pipe against being clobbered in the second pass by making sure`
	`549`	`+ // it's greater than any fd we are about to dup3() on top of.`
	`550`	`+ if pipe < len(fd) {`
	`551`	`+ fd1, _, err1 = RawSyscall(fcntl64Syscall, uintptr(pipe), F_DUPFD_CLOEXEC, 0)`
	`552`	`+ if err1 != 0 {`
	`553`	`+ goto childerror`
`565`	`554`	`}`
	`555`	`+ pipe = int(fd1)`
`566`	`556`	`}`
`567`	`557`
`568`	`558`	`// Pass 2: dup fd[i] down onto i.`

0 commit comments

Comments

(0)

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit c8c93ef

File tree

1 file changed

1 file changed

`‎src/syscall/exec_linux.go`

0 commit comments