Perhaps it makes sense to consolidate this code since it's repeated twice. Something like

seccompFd, err := recvSeccompFd(childPid, sync.Fd)

maybe?

Yeap, good call. I'm moving only these bits to the new function and not the if p.config.Config.Seccomp.ListenerPath == "" part as we need p for that and it would be called with p being different structs. Also, IMHO, semantically seems better to keep both things separate

1. \n should be removed.
2. Please to use %w for error.

3. I think net.Dial already includes path/address in the error message, as well as the error (dial), so there's no need to wrap it.

Cool, returning err then

I think net.Dial already includes path/address in the error message, as well as the error (dial), so there's no need to wrap it.

Correct:

$ cat main.go 
package main

import (
	"fmt"
	"net"
)

func main() {
	_, err := net.Dial("unix", "/fdsjkfhdjk")
	if err != nil {
		fmt.Printf("%s\n", err)
	}
}
$ go run main.go 
dial unix /fdsjkfhdjk: connect: no such file or directory

Is there a reason for this restriction? I can imagine how it might be unsafe (or unwise) to do this on paper, but is it really necessary to disallow this within runc?

Yes, basically it will hang for ever if we allow it. We need the write syscall to write the seccomp fd plain number over the pipe to the parent (we write to the pipe here), then the parent gets the fd with pifd_getfd and sends it over the unix socket to the seccomp agent.

If the write syscall has the notify action, we can't never send a msg to the parent, which will mean the seccomp agent won't get the seccomp fd and the write will be blocked forever.

However read/close (functions that we also use to sync with parent via the pipe) are not restricted as it is possible to send the seccomp fd to the parent, the seccomp agent will get the seccomp fd and the first few notifications it will get if read/close are handled are for the ones done in syncParentSeccomp() (and quite trivially allow to proceed syscalls on fds that are not known by the agent, for example).

Please note the parent is able to proceed, while we are blocked in read/close, because the parent is another process and not subject to the seccomp filter.

I will add a comment to explain this in the code, if that sounds good to you @cyphar

@cyphar Added a comment explaining this here

Ah of course. Yeah that's a bit sad :/ -- but we can always figure out some workaround for this in the future. (I wonder how the LXC folks worked around this problem -- though I think they don't have a similar write-based synchronisation model since they're written in C which means you don't need to do so much busy work.)

@cyphar I've asked Christian Brauner in the past. It seems they use clone3 sharing the fds table and use a fixed number for the seccomp notify fd. Then, they get it with pidfd_getfd().

Not sure how LXC uses a fixed fd number for seccomp notify, though.

You could just pick an arbitrarily high fd and use dup3 to clone it to the right fd number. It might be interesting to look into whether we could do it with pidfd_getfd though since we'd need to have a fallback for quite a while, it'd probably complicate the code a bit too much...

Oh, right, that was easy. Yeah, for runc I think is simpler to use the pipe, that is the main sync mechanism used tree-wide. We are using pidfd_getfd here, though: we sync with the parent to let them know when the fd is ready and send the fd plain number, then the parent gets it with pidfd_getfd

Ah, I forgot that we are requiring Linux 5.7 or newer for the feature -- pidfd_getfd isn't available for the earlier kernels with seccomp notify support.

And 5.7 is also enforced by libseccomp-golang, that only supports notification with tsync (requires libseccomp api level 6). This is as the vast majority of go programs will need tsync too. Here is the conversation we had with upstream about this when writing the patches for libseccomp-golang

Before 5.7, notify and tsync where mutually exclusive (torvalds/linux@7a0df7f). From Linux 5.7 introduced a new API to enable seccomp notification with tsync together in this commit torvalds/linux@5189149.