Skip to content
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.

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

x/sys/unix: ParseSocketControlMessage error invalid argument #56384

Closed
v-byte-cpu opened this issue Oct 23, 2022 · 6 comments
Closed

x/sys/unix: ParseSocketControlMessage error invalid argument #56384

v-byte-cpu opened this issue Oct 23, 2022 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@v-byte-cpu
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.19.2 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
...
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOVERSION="go1.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2418195348=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I tried to pass file descriptor over unix domain socket

for sending process:

// fd int
rights := unix.UnixRights(fd)
unix.Sendmsg(socketFd, nil, rights, nil, 0)

for receiving process:

// receive socket control message
b := make([]byte, unix.CmsgSpace(4))
if _, _, _, _, err = unix.Recvmsg(socketFd, nil, b, 0); err != nil {
	return
}

// parse socket control message
cmsgs, err := unix.ParseSocketControlMessage(b)
if err != nil {
	return 0, fmt.Errorf("parse socket control message: %w", err)
}

fds, err := unix.ParseUnixRights(&cmsgs[0])
...

What did you expect to see?

unix.ParseSocketControlMessage successfully parses the message

What did you see instead?

unix.ParseSocketControlMessage fails with invalid argument error

I performed a research and found out that the problem occurred after this commit golang/sys@87e55d7

@gopherbot gopherbot added this to the Unreleased milestone Oct 23, 2022
@ianlancetaylor
Copy link
Member

Can you show us a complete standalone program that demonstrates the bug? Thanks.

@seankhliao seankhliao changed the title x/sys: unix.ParseSocketControlMessage error invalid argument x/sys/unix ParseSocketControlMessage error invalid argument Oct 24, 2022
@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 24, 2022
@v-byte-cpu
Copy link
Author

sure, I will provide it soon.

@ianlancetaylor ianlancetaylor changed the title x/sys/unix ParseSocketControlMessage error invalid argument x/sys/unix: ParseSocketControlMessage error invalid argument Oct 24, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 24, 2022
@v-byte-cpu
Copy link
Author

example with nil payload in unix.Sendmsg and unix.Write

output

with golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664:

child: pipeFd 4
child: opened tmp file with fd 3
child: Sendmsg success
child: Write success
parent: recvmsg success
parent: read success [hello]
parent: parseSocketControlMessage success
parent: ParseUnixRights success
parent: got fd 5

with golang.org/x/sys v0.0.0-20220624220833-87e55d714810:

child: pipeFd 4
child: opened tmp file with fd 3
child: Sendmsg success
child: Write success
parent: recvmsg success
parent: read success [ello]
panic: invalid argument

goroutine 1 [running]:
main.parent()
	/home/mind/workspace/unixsock-example/main.go:66 +0x64b
main.main()
	/home/mind/workspace/unixsock-example/main.go:22 +0x5c

As you can see from the output in line parent: read success [ello], I guess that recvmsg has started to read an additional byte (in case of nil payload slice)

code

package main

import (
	"fmt"
	"os"
	"os/exec"
	"strconv"

	"golang.org/x/sys/unix"
)

const dataPayload = "hello"

func main() {
	if len(os.Args) > 2 && os.Args[1] == "child" {
		pipeFd, err := strconv.Atoi(os.Args[2])
		if err != nil {
			panic(err)
		}
		child(pipeFd)
	} else {
		parent()
	}
}

func parent() {
	fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0)
	if err != nil {
		panic(err)
	}
	defer unix.Close(fds[0])
	defer unix.Close(fds[1])

	parentPipeFd := fds[0]
	// set clo_exec flag on parent file descriptor
	_, err = unix.FcntlInt(uintptr(parentPipeFd), unix.F_SETFD, unix.FD_CLOEXEC)
	if err != nil {
		panic(err)
	}

	cmd := exec.Command("/proc/self/exe", "child", strconv.Itoa(fds[1]))
	cmd.Stdin = os.Stdin
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr
	if err = cmd.Start(); err != nil {
		panic(err)
	}

	// receive socket control message
	b := make([]byte, unix.CmsgSpace(4))
	_, _, _, _, err = unix.Recvmsg(parentPipeFd, nil, b, 0)
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: recvmsg success")
	data := make([]byte, len(dataPayload))
	_, err = unix.Read(parentPipeFd, data)
	if err != nil {
		panic(err)
	}
	fmt.Printf("parent: read success [%s]\n", data)

	// parse socket control message
	cmsgs, err := unix.ParseSocketControlMessage(b)
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: parseSocketControlMessage success")
	receivedFds, err := unix.ParseUnixRights(&cmsgs[0])
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: ParseUnixRights success")
	receivedFd := receivedFds[0]
	defer unix.Close(receivedFd)
	fmt.Printf("parent: got fd %d\n", receivedFd)

	if err = cmd.Wait(); err != nil {
		panic(err)
	}
}

func child(pipeFd int) {
	pipeFd, err := strconv.Atoi(os.Args[2])
	if err != nil {
		panic(err)
	}
	fmt.Printf("child: pipeFd %d\n", pipeFd)
	f, err := os.CreateTemp("", "tmpfile-")
	if err != nil {
		panic(err)
	}
	//defer os.Remove(f.Name())
	//defer f.Close()
	fd := int(f.Fd())
	fmt.Printf("child: opened tmp file with fd %d\n", fd)
	rights := unix.UnixRights(fd)
	if err := unix.Sendmsg(pipeFd, nil, rights, nil, 0); err != nil {
		panic(err)
	}
	fmt.Println("child: Sendmsg success")

	_, err = unix.Write(pipeFd, []byte(dataPayload))
	if err != nil {
		panic(err)
	}
	fmt.Println("child: Write success")

}
example with nil payload in unix.Sendmsg and without unix.Write

output

with golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664:

child: pipeFd 4
child: opened tmp file with fd 3
child: Sendmsg success
parent: recvmsg success
parent: parseSocketControlMessage success
parent: ParseUnixRights success
parent: got fd 5

with golang.org/x/sys v0.0.0-20220624220833-87e55d714810:

it hangs with the following output

child: pipeFd 4
child: opened tmp file with fd 3
child: Sendmsg success

So it confirms the assumption that recvmsg tries to read additional bytes from unix socket descriptor (in case of nil payload slice)

code

package main

import (
	"fmt"
	"os"
	"os/exec"
	"strconv"

	"golang.org/x/sys/unix"
)

const dataPayload = "hello"

func main() {
	if len(os.Args) > 2 && os.Args[1] == "child" {
		pipeFd, err := strconv.Atoi(os.Args[2])
		if err != nil {
			panic(err)
		}
		child(pipeFd)
	} else {
		parent()
	}
}

func parent() {
	fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0)
	if err != nil {
		panic(err)
	}
	defer unix.Close(fds[0])
	defer unix.Close(fds[1])

	parentPipeFd := fds[0]
	// set clo_exec flag on parent file descriptor
	_, err = unix.FcntlInt(uintptr(parentPipeFd), unix.F_SETFD, unix.FD_CLOEXEC)
	if err != nil {
		panic(err)
	}

	cmd := exec.Command("/proc/self/exe", "child", strconv.Itoa(fds[1]))
	cmd.Stdin = os.Stdin
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr
	if err = cmd.Start(); err != nil {
		panic(err)
	}

	// receive socket control message
	b := make([]byte, unix.CmsgSpace(4))
	_, _, _, _, err = unix.Recvmsg(parentPipeFd, nil, b, 0)
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: recvmsg success")
	//data := make([]byte, len(dataPayload))
	//_, err = unix.Read(parentPipeFd, data)
	//if err != nil {
	//	panic(err)
	//}
	//fmt.Printf("parent: read success [%s]\n", data)

	// parse socket control message
	cmsgs, err := unix.ParseSocketControlMessage(b)
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: parseSocketControlMessage success")
	receivedFds, err := unix.ParseUnixRights(&cmsgs[0])
	if err != nil {
		panic(err)
	}
	fmt.Println("parent: ParseUnixRights success")
	receivedFd := receivedFds[0]
	defer unix.Close(receivedFd)
	fmt.Printf("parent: got fd %d\n", receivedFd)

	if err = cmd.Wait(); err != nil {
		panic(err)
	}
}

func child(pipeFd int) {
	pipeFd, err := strconv.Atoi(os.Args[2])
	if err != nil {
		panic(err)
	}
	fmt.Printf("child: pipeFd %d\n", pipeFd)
	f, err := os.CreateTemp("", "tmpfile-")
	if err != nil {
		panic(err)
	}
	//defer os.Remove(f.Name())
	//defer f.Close()
	fd := int(f.Fd())
	fmt.Printf("child: opened tmp file with fd %d\n", fd)
	rights := unix.UnixRights(fd)
	if err := unix.Sendmsg(pipeFd, nil, rights, nil, 0); err != nil {
		panic(err)
	}
	fmt.Println("child: Sendmsg success")

	//_, err = unix.Write(pipeFd, []byte(dataPayload))
	//if err != nil {
	//	panic(err)
	//}
	//fmt.Println("child: Write success")

}

@rittneje
Copy link

The bug is here.
https://github.com/golang/sys/blob/95e765b1cc43ac521bd4fd501e00774e34401449/unix/syscall_linux.go#L1552-L1557

When the code got refactored in 87e55d714810 for RecvmsgBuffers, this change was not made correctly. It is missing the line iov = iova[:].

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/445255 mentions this issue: unix: in Linux sendmsgN actually send one normal byte

@ianlancetaylor
Copy link
Member

@v-byte-cpu thanks for the test case. @rittneje thanks for pointing out the error. I sent CL 445255 to fix this.

@golang golang locked and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants