On Tue, Aug 31, 2021 at 9:17 PM Michael Tokarev <
mjt@tls.msk.ru> wrote:
31.08.2021 01:54, Michael Tokarev wrote:
> Linux kernel can return size of af_unix socket to be
> one byte larger than sockaddr_un structure - adding
> the trailing zero byte.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> Fixes: 4cfd970ec188558daa6214f26203fe553fb1e01f (first in 6.1.0)
> Cc: qemu-stable@nongnu.org
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index f2f3676d1f..83926dc2bc 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1345,8 +1345,9 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
> SocketAddress *addr;
> struct sockaddr_un *su = (struct sockaddr_un *)sa;
>
> + /* kernel might have added \0 terminator to non-abstract socket */
> assert(salen >= sizeof(su->sun_family) + 1 &&
> - salen <= sizeof(struct sockaddr_un));
> + salen <= sizeof(struct sockaddr_un) + su->sun_path[0] ? 1 : 0);
>
> addr = g_new0(SocketAddress, 1);
> addr->type = SOCKET_ADDRESS_TYPE_UNIX;
Actually, this is not sufficient.
While this change fixes one issue (the famous trailing null byte \0),
the actual assertion failure occurs because salen = 2, ie, too SMALL,
not too large.
So it looks like libvirt provides an unnamed socket there, --
maybe from a socketpair(2)?
Yes
Ok, I guess it should still check for salen >= sizeof(su->sun_family)
and then modify if (salen > sizeof(su->sun_family) && !su->sun_path[0]) {
Hwell..
hmm, too bad we didn't catch it during RC!
(strange that it seems to hit Debian libvirt/virt-manager users and apparently not on Fedora)