Bug #86613 for Socket: Socket.xs does not handle correctly UNIX domain sockets on *BSD (at least FreeBSD)

This ticket is an updated copy (with updated patch) of https://rt.perl.org/rt3/Ticket/Display.html?id=67298 due to Tony demand. Bug with UNIX domain sockets on FreeBSD (at least 7.0 until 9.1 RELEASE). The unpack_sockaddr_un Socket.xs does not handle correctly struct sockaddr_un under FreeBSD in some cases. It returns errors like: Bad arg length for Socket::unpack_sockaddr_un, length is 16, should be 106 at ... When the system returns a struct sockaddr_un in getpeername(), it returns the same structure the server gives to bind(). In some cases, servers (X for /tmp/.X11-unix/X0 and devd for /var/run/devd.pipe, for example) give a shortest structure than sizeof(struct sockaddr_un) but with a correct sun_len field. In these cases, sun_path may not be '\0' terminated. Then getpeername() returns a structure that Socket.xs unpack_sockaddr_un() function thinks wrong sized. But it is not. One can reproduce the problem with the following simple server and client code. Server side : --------------------------------------- use strict; use Socket; my $name = $ARGV[0] or die "usage: $0 server_socket_file"; #my $sun = sockaddr_un($name); # Use a shortest sockaddr_un as FreeBSD allows it for its daemons my $sun = pack('CCA*', length($name)+2, 1, $name); socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket: $!\n"; bind($fh, $sun) or die "bind: $!\n"; listen($fh, 1) or die "listen: $!\n"; while (1) { accept(my $new_fh, $fh) or die "accept: $!\n"; sleep 1; } --------------------------------------- Client side (note that /var/run/devd.pipe socket is always available on standard FreeBSD installation): --------------------------------------- use strict; use Socket; my $name = $ARGV[0] // '/var/run/devd.pipe'; warn "*** Connecting to $name\n"; my $sun = sockaddr_un($name); socket(my $fh, PF_UNIX, SOCK_STREAM, 0) or die "socket: $!\n"; connect($fh, $sun) or die "connect: $!\n"; my $sockaddr = getpeername($fh) or die "getpeername: $!\n"; my $warn; ($warn = $sockaddr) =~ s/([\x00-\x19\x7f-\xff])/sprintf("<%02x>",ord$1)/ge; warn "hexdump: $warn--\n"; warn "real length: " . length($sockaddr) . "\n"; warn "sun_len field: " . unpack('C', $sockaddr) . "\n"; my $file = unpack_sockaddr_un($sockaddr); warn "unpack: $file--\n"; close $fh; --------------------------------------- I join a patch that correct the problem on FreeBSD. I tested it on Linux with no regression. I think the same problem occurs for each implementation that define sun_len in sockaddr_un struct. That's why the patch does not focus on FreeBSD but on the presence (or not) of this field. Don't hesitate to contact me to do some tests if you need... Best regards, Max.

--- Socket-2.010/Socket.xs.orig 2013-07-01 20:09:38.049749348 +0200 +++ Socket-2.010/Socket.xs 2013-07-01 21:54:49.311744378 +0200 @@ -717,8 +717,8 @@ STRLEN sockaddrlen; char * sun_ad = SvPVbyte(sun_sv,sockaddrlen); int addr_len; -# ifdef __linux__ - /* On Linux sockaddrlen on sockets returned by accept, recvfrom, +# if defined(__linux__) || defined(HAS_SOCKADDR_SA_LEN) + /* On Linux or *BSD sockaddrlen on sockets returned by accept, recvfrom, getpeername and getsockname is not equal to sizeof(addr). */ if (sockaddrlen < sizeof(addr)) { Copy(sun_ad, &addr, sockaddrlen, char); @@ -726,6 +726,12 @@ } else { Copy(sun_ad, &addr, sizeof(addr), char); } +# ifdef HAS_SOCKADDR_SA_LEN + /* In this case, sun_len must be checked */ + if (sockaddrlen != addr.sun_len) + croak("Invalid arg sun_len field for %s, length is %"UVuf", but sun_len is %"UVuf, + "Socket::unpack_sockaddr_un", (UV)sockaddrlen, (UV)addr.sun_len); +# endif # else if (sockaddrlen != sizeof(addr)) croak("Bad arg length for %s, length is %"UVuf", should be %"UVuf, @@ -736,14 +742,24 @@ if (addr.sun_family != AF_UNIX) croak("Bad address family for %s, got %d, should be %d", "Socket::unpack_sockaddr_un", addr.sun_family, AF_UNIX); - +# ifdef __linux__ if (addr.sun_path[0] == '\0') { /* Linux-style abstract socket address begins with a nul * and can contain nuls. */ addr_len = (char *)&addr - (char *)&(addr.sun_path) + sockaddrlen; - } else { + } else +# endif + { +# if defined(HAS_SOCKADDR_SA_LEN) + /* On *BSD sun_path not always ends with a '\0' */ + int maxlen = addr.sun_len - 2; /* should use offsetof(struct sockaddr_un, sun_path) instead of 2 */ + if (maxlen > (int)sizeof(addr.sun_path)) + maxlen = (int)sizeof(addr.sun_path); +# else + const int maxlen = (int)sizeof(addr.sun_path); +# endif for (addr_len = 0; addr.sun_path[addr_len] - && addr_len < (int)sizeof(addr.sun_path); addr_len++); + && addr_len < maxlen; addr_len++); } ST(0) = sv_2mortal(newSVpvn(addr.sun_path, addr_len));