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

syscall: support for abstract unix domain socket with len(name) == len(sa.raw.Path) #21965

Closed
N-Bz opened this issue Sep 21, 2017 · 3 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@N-Bz
Copy link
Contributor

N-Bz commented Sep 21, 2017

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

go version go1.9 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOOS="linux" GOARCH="amd64"
But the issue seems to be present on every linux system (also tested on armv7)

What did you do?

I have a C server process which creates an unix domain socket with the following code:

/* beginning of function omitted */
struct sockaddr_un addr_un;
memset(&addr_un, 0, sizeof(addr_un));
snprintf(addr_un.sun_path, sizeof(addr_un.sun_path), "@something");
addr_un.sun_path[0] = '\0';
bind(sockfd, (struct sockaddr *)&addr_un, sizeof(addr_un));
/* end of function omitted */

By calling the bind() function with the full sizeof(addr_un) as the size, it creates an abstract unix socket whose name is actually 108 bytes long (one starting null byte, the socket name, and more null bytes as padding at the end). While not a good practice, that kind of code does exists in the wild.

I tried to make net.Dial() connection to this server from a go program

What did you expect to see?

Obviously, net.Dial("unix", "@something") fails. This is expected behaviour because the abstract unix socket name length (10 in this case) does not match the server size (108), but I expected the following code to work:

package main

import "net"
import "fmt"

func dialFillZero(network, name string) (net.Conn, error) {
    raw := []byte(name)
    addr_b := make([]byte, 108)
    for i, c := range raw {
        addr_b[i] = c
    }
    addr := string(addr_b)
    return net.Dial(network, addr)
}

func main() {
    conn, err := dialFillZero("unix", "@something")
    if err != nil {
        fmt.Println("Error:", err)
        return
    }
    fmt.Println("OK")
}

What did you see instead?

This code also fails because the func (sa *SockaddrUnix) sockaddr() function in src/syscall/syscall_linux.go returns an EINVAL error if the sockaddr name is greater or equal to the length of the sockaddr raw path (declared as 108 for linux).

This limit makes sense for non-abstract unix domain sockets, as the path have to be a null terminated string, but as the abstract unix domain sockets does not have this limitation, it artificially forbids connection to any socket with len(name) == len(sa.raw.Path)

I made the following change to my go distribution, which seems to solve the problem without creating any other visible issues (all tests are still OK):

diff --git a/src/syscall/syscall_linux.go b/src/syscall/syscall_linux.go
index 3c7d378..1ee8979 100644
--- a/src/syscall/syscall_linux.go
+++ b/src/syscall/syscall_linux.go
@@ -295,7 +295,10 @@ func (sa *SockaddrInet6) sockaddr() (unsafe.Pointer, _Socklen, error) {
 func (sa *SockaddrUnix) sockaddr() (unsafe.Pointer, _Socklen, error) {
        name := sa.Name
        n := len(name)
-       if n >= len(sa.raw.Path) {
+       if n > len(sa.raw.Path) {
+               return nil, 0, EINVAL
+       }
+       if n == len(sa.raw.Path) && int8(name[0]) != '@' {
                return nil, 0, EINVAL
        }
        sa.raw.Family = AF_UNIX

If the patch seams reasonable, I can submit it properly to Gerrit.

@ianlancetaylor ianlancetaylor changed the title Support for abstract unix domain socket with len(name) == len(sa.raw.Path) syscall: support for abstract unix domain socket with len(name) == len(sa.raw.Path) Sep 21, 2017
@ianlancetaylor
Copy link
Contributor

Something along the lines of that patch looks fine, with a test case. Thanks.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 21, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Sep 21, 2017
@gopherbot
Copy link

Change https://golang.org/cl/66190 mentions this issue: syscall: allow abstract unix socket to use the full Path len

@gopherbot
Copy link

Change https://golang.org/cl/66333 mentions this issue: net: put new TestUnixgramLinuxAbstractLongName in linux_test.go file

gopherbot pushed a commit that referenced this issue Sep 28, 2017
Also changed name from TestUnix... to TestUnixgram....

Updates #21965

Change-Id: I2833110b77e9fe1b28d4a15feb3d70453ab98d3b
Reviewed-on: https://go-review.googlesource.com/66333
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
akimd added a commit to akimd/hyperkit that referenced this issue Nov 20, 2017
First pinata tried to let them in stateDir.  However, at least its
current name there (`com.docker.driver.amd64-linux`) is so long that
our socket names become invalid for the OS (plenty of pages about this
on the Internet, e.g. golang/go#21965).

Allow the user to decide where to store them.  Pinata uses this
feature have shorter socket path names.

Signed-off-by: Akim Demaille <akim.demaille@docker.com>
akimd added a commit to akimd/hyperkit that referenced this issue Nov 23, 2017
When moving to use hyperkit.go, Docker for Mac first tried to leave
them in stateDir.  However, its current name there
(`"com.docker.driver.amd64-linux"`) is so long that the socket names
become invalid for the OS (plenty of pages about this on the Internet,
e.g. golang/go#21965).

Allow the user to decide where to store them.  Docker for Mac uses
this feature have shorter socket path names.

Signed-off-by: Akim Demaille <akim.demaille@docker.com>
akimd added a commit to akimd/hyperkit that referenced this issue Dec 6, 2017
When moving to use hyperkit.go, Docker for Mac first tried to leave
them in stateDir.  However, its current name there
(com.docker.driver.amd64-linux) is so long that the socket names
become invalid for the OS (plenty of pages about this on the Internet,
e.g. golang/go#21965).

Allow the user to decide where to store them.  Docker-for-Mac uses
this feature to have shorter socket path names.

Signed-off-by: Akim Demaille <akim.demaille@docker.com>
@golang golang locked and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants