Compiling with clang-8 fails with:

    CC      util/qemu-sockets.o
  util/qemu-sockets.c: In function 'unix_connect_saddr':
  util/qemu-sockets.c:925:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
       strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  util/qemu-sockets.c: In function 'unix_listen_saddr':
  util/qemu-sockets.c:880:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
       strncpy(un.sun_path, path, sizeof(un.sun_path));
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Per the unix socket manpage:

  UNIX(7)

  Pathname sockets
  When binding a socket to a pathname, a few rules should be observed for maximum portability and ease of coding:
  *  The pathname in sun_path should be null-terminated.
  *  The length of the pathname, including the terminating null byte, should not exceed the size of sun_path.

Reduce the length of the unix socket path by 1 to hold the NUL byte.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/qemu-sockets.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 97050516900..935271d58c0 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -845,10 +845,10 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
         path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX", tmpdir);
     }
 
-    if (strlen(path) > sizeof(un.sun_path)) {
+    if (strlen(path) > sizeof(un.sun_path) - 1) {
         error_setg(errp, "UNIX socket path '%s' is too long", path);
         error_append_hint(errp, "Path must be less than %zu bytes\n",
-                          sizeof(un.sun_path));
+                          sizeof(un.sun_path) - 1);
         goto err;
     }
 
@@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, path, sizeof(un.sun_path));
+    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
 
     if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
         error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
@@ -913,16 +913,16 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
         return -1;
     }
 
-    if (strlen(saddr->path) > sizeof(un.sun_path)) {
+    if (strlen(saddr->path) > sizeof(un.sun_path) - 1) {
         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));
+                          sizeof(un.sun_path) - 1);
         goto err;
     }
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
+    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
 
     /* connect to peer */
     do {
-- 
2.20.1

On Wed, Apr 03, 2019 at 02:16:20PM +0200, Philippe Mathieu-Daudé wrote:
> Compiling with clang-8 fails with:
> 
>     CC      util/qemu-sockets.o
>   util/qemu-sockets.c: In function 'unix_connect_saddr':
>   util/qemu-sockets.c:925:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
>        strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   util/qemu-sockets.c: In function 'unix_listen_saddr':
>   util/qemu-sockets.c:880:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
>        strncpy(un.sun_path, path, sizeof(un.sun_path));
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Per the unix socket manpage:
> 
>   UNIX(7)
> 
>   Pathname sockets
>   When binding a socket to a pathname, a few rules should be observed for maximum portability and ease of coding:
>   *  The pathname in sun_path should be null-terminated.
>   *  The length of the pathname, including the terminating null byte, should not exceed the size of sun_path.
> 
> Reduce the length of the unix socket path by 1 to hold the NUL byte.

Note it just says "should", not "must" here. IOW, there is no requirement
to NUL terminate and so we should not artifically require that at QEMU
level either. If mgmt apps want to have NUL termination then they can
just pass a shorter path to QEMU to start with.

I've proposed the fix for the warning you mention here:

  https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07759.html


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Wed, Apr 3, 2019 at 2:23 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Apr 03, 2019 at 02:16:20PM +0200, Philippe Mathieu-Daudé wrote:
> > Compiling with clang-8 fails with:
> >
> >     CC      util/qemu-sockets.o
> >   util/qemu-sockets.c: In function 'unix_connect_saddr':
> >   util/qemu-sockets.c:925:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
> >        strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
> >        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   util/qemu-sockets.c: In function 'unix_listen_saddr':
> >   util/qemu-sockets.c:880:5: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
> >        strncpy(un.sun_path, path, sizeof(un.sun_path));
> >        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Per the unix socket manpage:
> >
> >   UNIX(7)
> >
> >   Pathname sockets
> >   When binding a socket to a pathname, a few rules should be observed for maximum portability and ease of coding:
> >   *  The pathname in sun_path should be null-terminated.
> >   *  The length of the pathname, including the terminating null byte, should not exceed the size of sun_path.
> >
> > Reduce the length of the unix socket path by 1 to hold the NUL byte.
>
> Note it just says "should", not "must" here. IOW, there is no requirement
> to NUL terminate and so we should not artifically require that at QEMU
> level either. If mgmt apps want to have NUL termination then they can
> just pass a shorter path to QEMU to start with.
>
> I've proposed the fix for the warning you mention here:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07759.html

Oh I missed it, thanks for pointing it.

Regards,

Phil.