[Qemu-devel] [PATCH v2] sockets: improve error reporting if UNIX socket path is too long

[Daniel P. Berrange]

The 'struct sockaddr_un' only allows 108 bytes for the socket
path.

If the user supplies a path, QEMU uses snprintf() to silently
truncate it when too long. This is undesirable because the user
will then be unable to connect to the path they asked for.

If the user doesn't supply a path, QEMU builds one based on
TMPDIR, but if that leads to an overlong path, it mistakenly
uses error_setg_errno() with a stale errno value, because
snprintf() does not set errno on truncation.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 util/qemu-sockets.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index d8183f7..d565499 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -845,6 +845,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
 {
     struct sockaddr_un un;
     int sock, fd;
+    size_t pathlen;
 
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
@@ -854,15 +855,25 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    if (saddr->path && strlen(saddr->path)) {
-        snprintf(un.sun_path, sizeof(un.sun_path), "%s", saddr->path);
+    pathlen = saddr->path ? strlen(saddr->path) : 0;
+    if (pathlen != 0) {
+        if (pathlen > sizeof(un.sun_path)) {
+            error_setg(errp, "UNIX socket path '%s' is too long", saddr->path);
+            error_append_hint(errp, "Path must be less than %zu bytes\n",
+                              sizeof(un.sun_path));
+            goto err;
+        }
+        strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
     } else {
+        const char *template = "qemu-socket-XXXXXX";
         const char *tmpdir = getenv("TMPDIR");
         tmpdir = tmpdir ? tmpdir : "/tmp";
-        if (snprintf(un.sun_path, sizeof(un.sun_path), "%s/qemu-socket-XXXXXX",
-                     tmpdir) >= sizeof(un.sun_path)) {
-            error_setg_errno(errp, errno,
-                             "TMPDIR environment variable (%s) too large", 
tmpdir);
+        if (snprintf(un.sun_path, sizeof(un.sun_path), "%s/%s",
+                     tmpdir, template) >= sizeof(un.sun_path)) {
+            error_setg(errp,
+                       "TMPDIR environment variable (%s) too large", tmpdir);
+            error_append_hint(errp, "Path must be less than %zu bytes\n",
+                              sizeof(un.sun_path) - strlen(template) - 1);
             goto err;
         }
 
-- 
2.9.3