[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] sockets: report error if UNIX socket path is to
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] sockets: report error if UNIX socket path is too long |
Date: |
Wed, 24 May 2017 11:00:05 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
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 <address@hidden>
>> ---
>> 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