82 lines
3.1 KiB
Diff
82 lines
3.1 KiB
Diff
From 118d527f2e4baec5fe8060b22a6212468b8e4d3f Mon Sep 17 00:00:00 2001
|
|
From: Michael Tokarev <mjt@tls.msk.ru>
|
|
Date: Wed, 1 Sep 2021 16:16:24 +0300
|
|
Subject: [PATCH] qemu-sockets: fix unix socket path copy (again)
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Commit 4cfd970ec188558daa6214f26203fe553fb1e01f added an
|
|
assert which ensures the path within an address of a unix
|
|
socket returned from the kernel is at least one byte and
|
|
does not exceed sun_path buffer. Both of this constraints
|
|
are wrong:
|
|
|
|
A unix socket can be unnamed, in this case the path is
|
|
completely empty (not even \0)
|
|
|
|
And some implementations (notable linux) can add extra
|
|
trailing byte (\0) _after_ the sun_path buffer if we
|
|
passed buffer larger than it (and we do).
|
|
|
|
So remove the assertion (since it causes real-life breakage)
|
|
but at the same time fix the usage of sun_path. Namely,
|
|
we should not access sun_path[0] if kernel did not return
|
|
it at all (this is the case for unnamed sockets),
|
|
and use the returned salen when copyig actual path as an
|
|
upper constraint for the amount of bytes to copy - this
|
|
will ensure we wont exceed the information provided by
|
|
the kernel, regardless whenever there is a trailing \0
|
|
or not. This also helps with unnamed sockets.
|
|
|
|
Note the case of abstract socket, the sun_path is actually
|
|
a blob and can contain \0 characters, - it should not be
|
|
passed to g_strndup and the like, it should be accessed by
|
|
memcpy-like functions.
|
|
|
|
Fixes: 4cfd970ec188558daa6214f26203fe553fb1e01f
|
|
Fixes: http://bugs.debian.org/993145
|
|
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
|
|
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
|
|
CC: qemu-stable@nongnu.org
|
|
---
|
|
util/qemu-sockets.c | 13 +++++--------
|
|
1 file changed, 5 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
|
|
index f2f3676d1f..c5043999e9 100644
|
|
--- a/util/qemu-sockets.c
|
|
+++ b/util/qemu-sockets.c
|
|
@@ -1345,25 +1345,22 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
|
|
SocketAddress *addr;
|
|
struct sockaddr_un *su = (struct sockaddr_un *)sa;
|
|
|
|
- assert(salen >= sizeof(su->sun_family) + 1 &&
|
|
- salen <= sizeof(struct sockaddr_un));
|
|
-
|
|
addr = g_new0(SocketAddress, 1);
|
|
addr->type = SOCKET_ADDRESS_TYPE_UNIX;
|
|
+ salen -= offsetof(struct sockaddr_un, sun_path);
|
|
#ifdef CONFIG_LINUX
|
|
- if (!su->sun_path[0]) {
|
|
+ if (salen > 0 && !su->sun_path[0]) {
|
|
/* Linux abstract socket */
|
|
- addr->u.q_unix.path = g_strndup(su->sun_path + 1,
|
|
- salen - sizeof(su->sun_family) - 1);
|
|
+ addr->u.q_unix.path = g_strndup(su->sun_path + 1, salen - 1);
|
|
addr->u.q_unix.has_abstract = true;
|
|
addr->u.q_unix.abstract = true;
|
|
addr->u.q_unix.has_tight = true;
|
|
- addr->u.q_unix.tight = salen < sizeof(*su);
|
|
+ addr->u.q_unix.tight = salen < sizeof(su->sun_path);
|
|
return addr;
|
|
}
|
|
#endif
|
|
|
|
- addr->u.q_unix.path = g_strndup(su->sun_path, sizeof(su->sun_path));
|
|
+ addr->u.q_unix.path = g_strndup(su->sun_path, salen);
|
|
return addr;
|
|
}
|
|
#endif /* WIN32 */
|