Re: [Libguestfs] [libnbd PATCH] connect: Better handling of long socket names

Copy various Unix socket handling techniques from nbdkit's nbd plugin: Silently truncating a socket name rather than issuing an error message can confuse users. No need to do an explicit memset if the compiler does it for us via an initializer. No need to use strncpy() which does wasted effort on short names, when we can use memcpy() given that we already checked length in order to detect truncation. Linux does not require a trailing NUL byte in sockaddr_un, so we can allow names one byte longer. Not entirely fixed: although the proposed NBD URI document mentions that URIs can start with a leading (encoded) NUL byte to access the Linux abstract socket namespace, our use of strlen(sun.sun_path) would truncate the trailing bytes - but to support abstract sockets in general, we'd need to add h->unixlen alongside the existing h->unixsocket. --- generator/states-connect.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/generator/states-connect.c b/generator/states-connect.c index a410e34..014f6bb 100644 --- a/generator/states-connect.c +++ b/generator/states-connect.c @@ -86,15 +86,20 @@ } CONNECT_UNIX.START: - struct sockaddr_un sun; + struct sockaddr_un sun = { .sun_family = AF_UNIX }; socklen_t len; + size_t socklen; assert (h->unixsocket != NULL); - sun.sun_family = AF_UNIX; - memset (sun.sun_path, 0, sizeof (sun.sun_path)); - strncpy (sun.sun_path, h->unixsocket, sizeof (sun.sun_path) - 1); - len = sizeof (sun.sun_family) + strlen (sun.sun_path) + 1; + socklen = strlen (h->unixsocket); + if (socklen > sizeof sun.sun_path) { + set_error (ENAMETOOLONG, "socket name too long: %s", h->unixsocket); + SET_NEXT_STATE (%.DEAD); + return -1; + } + memcpy (sun.sun_path, h->unixsocket, socklen); + len = sizeof sun; memcpy (&h->connaddr, &sun, len); h->connaddrlen = len; --