Re: [PATCH] util: fix abstract socket path copy

[Michael Tokarev]

31.08.2021 01:06, Michael Tokarev wrote:
...
> And this is the value used to be returned in the getsockname/getpeername
> calls.
>
> So this has nothing to do with socket being abstract or not. We asked for
> larger storage for the sockaddr structure, and the kernel was able to build
> one for us, including the trailing \0 byte. Currently kernel will only
> return +1 byte for non-abstract sockets, but this is an implementation
> detail. We asked for it. So we - I think - should account for this +1
> byte here.

And here's the note from man 7 unix:

BUGS
       When binding a socket to an address, Linux is one of the implementations 
that appends a null termina‐
       tor if none is supplied in sun_path.  In most cases this is 
unproblematic: when the socket address is
       retrieved, it will be one byte longer than that supplied when the socket 
was bound.   However,  there
       is  one case where confusing behavior can result: if 108 non-null bytes 
are supplied when a socket is
       bound,  then  the  addition  of  the  null  terminator  takes  the  
length  of  the  pathname  beyond
       sizeof(sun_path).   Consequently, when retrieving the socket address 
(for example, via accept(2)), if
       the input addrlen argument for the retrieving call is specified as 
sizeof(struct  sockaddr_un),  then
       the returned address structure won't have a null terminator in sun_path.

So how about this:

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index f2f3676d1f..83926dc2bc 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1345,8 +1345,9 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage 
*sa,
     SocketAddress *addr;
     struct sockaddr_un *su = (struct sockaddr_un *)sa;

+    /* kernel might have added \0 terminator to non-abstract socket */
     assert(salen >= sizeof(su->sun_family) + 1 &&
-           salen <= sizeof(struct sockaddr_un));
+           salen <= sizeof(struct sockaddr_un) + su->sun_path[0] ? 1 : 0);

     addr = g_new0(SocketAddress, 1);
     addr->type = SOCKET_ADDRESS_TYPE_UNIX;

?

/mjt