sockets: report error if UNIX socket path is too long
PATCH 1/1 sockets: report error if UNIX socket path is too long
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.
--- util/qemu-sockets.c | 6 ++++++ 1 file changed, 6 insertions(+)
util/qemu-sockets.c +6 -0
@@ -855,6 +855,12 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,memset (&un,0 ,sizeof (un)); un.sun_family =AF_UNIX ;if (saddr->path &&strlen (saddr->path)) { +if (strlen (saddr->path) >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)); +return -1 ; + }snprintf (un.sun_path,sizeof (un.sun_path),"%s" , saddr->path); }else {const char *tmpdir =getenv ("TMPDIR" );
Signature · 1 line
2.9.3
Quoted 17 lines · Daniel P. Berrange
The 'struct
sockaddr_un' only allows 108 bytes for the socket path. Currently QEMU usessnprintf()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 <[email protected]>--- util/qemu-sockets.c | 6 ++++++ 1 file changed, 6 insertions(+)util/qemu-sockets.c +0
-0
util/qemu-sockets.c static int unix_listen_saddr(UnixSocketAddress *saddr, · line 858
Quoted 4 lines
util/qemu-sockets.c +1
-0@@ -855,6 +855,12 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,memset (&un,0 ,sizeof (un)); un.sun_family =AF_UNIX ;if (saddr->path &&strlen (saddr->path)) { +if (strlen (saddr->path) >sizeof (un.sun_path)) {
Feels odd to be computing strlen(saddr->path) multiple times instead of storing it in a temporary.
util/qemu-sockets.c static int unix_listen_saddr(UnixSocketAddress *saddr, · line 864
Quoted 6 lines
util/qemu-sockets.c +5
-0+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)); +return -1 ; + }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.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 14 lines · Eric Blake
Quoted 12 lines · Daniel P. Berrange
The 'struct
sockaddr_un' only allows 108 bytes for the socket path. Currently QEMU usessnprintf()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 <[email protected]>--- util/qemu-sockets.c | 6 ++++++ 1 file changed, 6 insertions(+)
Quoted 6 lines
Quoted 1 line
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().
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org