Skip to content

Commit 118d527

Browse files
Michael Tokarevroot
authored andcommitted
qemu-sockets: fix unix socket path copy (again)
Commit 4cfd970 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: 4cfd970 Fixes: http://bugs.debian.org/993145 Signed-off-by: Michael Tokarev <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> Reviewed-by: Marc-André Lureau <[email protected]> CC: [email protected]
1 parent 935efca commit 118d527

File tree

1 file changed

+5
-8
lines changed

1 file changed

+5
-8
lines changed

util/qemu-sockets.c

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,25 +1345,22 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
13451345
SocketAddress *addr;
13461346
struct sockaddr_un *su = (struct sockaddr_un *)sa;
13471347

1348-
assert(salen >= sizeof(su->sun_family) + 1 &&
1349-
salen <= sizeof(struct sockaddr_un));
1350-
13511348
addr = g_new0(SocketAddress, 1);
13521349
addr->type = SOCKET_ADDRESS_TYPE_UNIX;
1350+
salen -= offsetof(struct sockaddr_un, sun_path);
13531351
#ifdef CONFIG_LINUX
1354-
if (!su->sun_path[0]) {
1352+
if (salen > 0 && !su->sun_path[0]) {
13551353
/* Linux abstract socket */
1356-
addr->u.q_unix.path = g_strndup(su->sun_path + 1,
1357-
salen - sizeof(su->sun_family) - 1);
1354+
addr->u.q_unix.path = g_strndup(su->sun_path + 1, salen - 1);
13581355
addr->u.q_unix.has_abstract = true;
13591356
addr->u.q_unix.abstract = true;
13601357
addr->u.q_unix.has_tight = true;
1361-
addr->u.q_unix.tight = salen < sizeof(*su);
1358+
addr->u.q_unix.tight = salen < sizeof(su->sun_path);
13621359
return addr;
13631360
}
13641361
#endif
13651362

1366-
addr->u.q_unix.path = g_strndup(su->sun_path, sizeof(su->sun_path));
1363+
addr->u.q_unix.path = g_strndup(su->sun_path, salen);
13671364
return addr;
13681365
}
13691366
#endif /* WIN32 */

0 commit comments

Comments
 (0)