Fix this warning when building with GCC9 on Fedora 30:
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: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-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:
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
util/qemu-sockets.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9705051690..4322652428 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -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);
@@ -922,7 +922,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));
+ strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
/* connect to peer */
do {
--
2.21.0
Le 30/04/2019 à 22:08, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> 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: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-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:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> util/qemu-sockets.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9705051690..4322652428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -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);
> @@ -922,7 +922,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));
> + strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
>
> /* connect to peer */
> do {
>
Your change reverts partially:
commit ad9579aaa16d5b385922d49edac2c96c79bcfb62
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Thu May 25 16:53:00 2017 +0100
sockets: improve error reporting if UNIX socket path is too long
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.
In solving this the code needed some refactoring to ensure we
don't pass 'un.sun_path' directly to any APIs which expect
NUL-terminated strings, because the path is not required to
be terminated.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20170525155300.22743-1-berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks,
Laurent
On 4/30/19 3:08 PM, Alistair Francis wrote: > Fix this warning when building with GCC9 on Fedora 30: > 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: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-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: > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > util/qemu-sockets.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 9705051690..4322652428 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -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); NACK. Linux allows you to use the full width of un.sun_path (a NUL terminator is required if you copy less than that, but not if you use the full width). Rather, we may need to mark path as a potential nonstring to silence the warning, or use memcpy instead of strncpy, or some other workaround. (Sadly, this is one of those odd places where strncpy is actually the right function to use, but there are so many other places where strncpy is used incorrectly that it has turned into a battle to use it here) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 4/30/19 3:25 PM, Eric Blake wrote: > On 4/30/19 3:08 PM, Alistair Francis wrote: >> Fix this warning when building with GCC9 on Fedora 30: >> 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: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-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: >> >> - strncpy(un.sun_path, path, sizeof(un.sun_path)); >> + strncpy(un.sun_path, path, sizeof(un.sun_path) - 1); > > NACK. Linux allows you to use the full width of un.sun_path (a NUL > terminator is required if you copy less than that, but not if you use > the full width). Rather, we may need to mark path as a potential > nonstring to silence the warning, or use memcpy instead of strncpy, or > some other workaround. (Sadly, this is one of those odd places where > strncpy is actually the right function to use, but there are so many > other places where strncpy is used incorrectly that it has turned into a > battle to use it here) We don't have control over un (that's from the libc system headers), but does adding the QEMU_NONSTRING attribute to our declaration of path serve to silence the warning? In short, I think most of this series should look at the use of the QEMU_NONSTRING macro, as that macro goes hand-in-hand with strncpy() for informing the compiler exactly when we know that we are copying something that has fixed length and may or may not be NUL-terminated. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Tue, Apr 30, 2019 at 2:16 PM Eric Blake <eblake@redhat.com> wrote: > > On 4/30/19 3:25 PM, Eric Blake wrote: > > On 4/30/19 3:08 PM, Alistair Francis wrote: > >> Fix this warning when building with GCC9 on Fedora 30: > >> 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: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-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: > >> > > >> - strncpy(un.sun_path, path, sizeof(un.sun_path)); > >> + strncpy(un.sun_path, path, sizeof(un.sun_path) - 1); > > > > NACK. Linux allows you to use the full width of un.sun_path (a NUL > > terminator is required if you copy less than that, but not if you use > > the full width). Rather, we may need to mark path as a potential > > nonstring to silence the warning, or use memcpy instead of strncpy, or > > some other workaround. (Sadly, this is one of those odd places where > > strncpy is actually the right function to use, but there are so many > > other places where strncpy is used incorrectly that it has turned into a > > battle to use it here) > > We don't have control over un (that's from the libc system headers), but > does adding the QEMU_NONSTRING attribute to our declaration of path > serve to silence the warning? I don't think that would fix it, I'll double check though. > > In short, I think most of this series should look at the use of the > QEMU_NONSTRING macro, as that macro goes hand-in-hand with strncpy() for > informing the compiler exactly when we know that we are copying > something that has fixed length and may or may not be NUL-terminated. I have changed the series to use memcpy() and QEMU_NONSTRING. I'll send a v2 out soon with all your comments addressed. Alistair > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org >
© 2016 - 2025 Red Hat, Inc.