[Libguestfs] [nbdkit PATCH] nbd: Fix gcc warning and off-by-one in socket name length

Eric Blake eblake at redhat.com
Thu Apr 5 15:27:10 UTC 2018


gcc 8 gripes (when using './configure --enable-gcc-warnings'):
nbd.c: In function 'nbd_open':
nbd.c:470:3: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation] strncpy (sock.sun_path, sockname, sizeof (sock.sun_path));

The warning is a false positive, given that we currently reject
names >= sizeof(sock.sun_path), and thus we are only ever copying
in a name that will include a trailing NUL.  However, note that
Linux permits a socket name to use the full width of sun_path (for
shorter names, you must either provide a trailing NUL or pass
something smaller than sizeof(struct sockaddr_un) to connect();
but for the full-length name, a trailing NUL is not required).

So, relax our off-by-one length restriction (we now permit the user
to pass a 108-byte socket name, instead of a limit of 107), at which
point the gcc complaint is no longer a false positive (we could
indeed be copying a string without its trailing NUL, even though
we know it works), then shut up gcc by using memcpy() instead of
strncpy(), relying on our earlier zero-initialization for supplying
any needed trailing NUL.  For convencience, we stick with the
simpler sizeof(struct sockaddr_un) instead of passing exact lengths
to connect().

[strncpy() is seldom the right function to use, because it does
not NUL-terminate on overflow, yet writes a full size bytes even
when the input string is shorter.  Initializing sockaddr_un is
one of the few places where it actually does what you want - too
bad newer gcc is now rendering even valid uses of strncpy as a
source of complaints]

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 plugins/nbd/nbd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 51de178..a4a1f12 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -100,7 +100,7 @@ nbd_config_complete (void)
     nbdkit_error ("you must supply the socket=<SOCKNAME> parameter after the plugin name on the command line");
     return -1;
   }
-  if (strlen (sockname) >= sizeof sock.sun_path) {
+  if (strlen (sockname) > sizeof sock.sun_path) {
     nbdkit_error ("socket file name too large");
     return -1;
   }
@@ -467,7 +467,8 @@ nbd_open (int readonly)
     nbdkit_error ("socket: %m");
     return NULL;
   }
-  strncpy (sock.sun_path, sockname, sizeof (sock.sun_path));
+  /* We already validated length during nbd_config_complete */
+  memcpy (sock.sun_path, sockname, strlen (sockname));
   if (connect (h->fd, (const struct sockaddr *) &sock, sizeof sock) < 0) {
     nbdkit_error ("connect: %m");
     goto err;
-- 
2.14.3




More information about the Libguestfs mailing list