Skip to content

Commit

Permalink
sockets: avoid string truncation warnings when copying UNIX path
Browse files Browse the repository at this point in the history
In file included from /usr/include/string.h:494,
                 from include/qemu/osdep.h:101,
                 from util/qemu-sockets.c:18:
In function ‘strncpy’,
    inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
/usr/include/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
/usr/include/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We are already validating the UNIX socket path length earlier in
the functions. If we save this string length when we first check
it, then we can simply use memcpy instead of strcpy later, avoiding
the gcc truncation warnings.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20190501145052.12579-1-berrange@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
  • Loading branch information
berrange authored and vivier committed May 3, 2019
1 parent 9176a58 commit 2d2023c
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions util/qemu-sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
int sock, fd;
char *pathbuf = NULL;
const char *path;
size_t pathlen;

sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
Expand All @@ -845,7 +846,8 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX", tmpdir);
}

if (strlen(path) > sizeof(un.sun_path)) {
pathlen = strlen(path);
if (pathlen > sizeof(un.sun_path)) {
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));
Expand Down Expand Up @@ -877,7 +879,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));
memcpy(un.sun_path, path, pathlen);

if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
Expand All @@ -901,6 +903,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
{
struct sockaddr_un un;
int sock, rc;
size_t pathlen;

if (saddr->path == NULL) {
error_setg(errp, "unix connect: no path specified");
Expand All @@ -913,7 +916,8 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
return -1;
}

if (strlen(saddr->path) > sizeof(un.sun_path)) {
pathlen = strlen(saddr->path);
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));
Expand All @@ -922,7 +926,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)

memset(&un, 0, sizeof(un));
un.sun_family = AF_UNIX;
strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
memcpy(un.sun_path, saddr->path, pathlen);

/* connect to peer */
do {
Expand Down

0 comments on commit 2d2023c

Please sign in to comment.