On 05/24/2017 10:54 AM, Eric Blake wrote: > On 05/24/2017 10:42 AM, Daniel P. Berrange wrote: >> The 'struct sockaddr_un' only allows 108 bytes for the socket >> path. Currently QEMU uses snprintf() and so silently truncates >> the socket path provided by the user. This is undesirable because >> the user will then be unable to connect to the path they asked >> for. This change makes QEMU bounds check and report an explicit >> error message. >> >> Signed-off-by: Daniel P. Berrange <berra...@redhat.com> >> --- >> util/qemu-sockets.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >>
>> snprintf(un.sun_path, sizeof(un.sun_path), "%s", saddr->path);
>
> Pre-existing, but now that we know things fit, isn't it faster to use
> strcpy (or strpcpy or strncpy) instead of the overhead of snprintf?
>
> You're on the right track, but it may be worth a v2 for further cleanups.
Oh, and while there, I noticed:
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);
but 'errno' is NOT required to be set when snprintf() returns a larger
value, so our error message is (likely) garbage. Better might be
calling error_setg_errno(errp, ENAMETOOLONG, ...), or just living with
the simpler error_setg().
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
