[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[qemu-xen staging] qemu-sockets: fix unix socket path copy (again)



commit 3ee93e456d08415cc1abe4110f6e0c9b0ead0bbc
Author:     Michael Tokarev <mjt@xxxxxxxxxx>
AuthorDate: Wed Sep 1 16:16:24 2021 +0300
Commit:     Michael Roth <michael.roth@xxxxxxx>
CommitDate: Tue Dec 14 12:55:36 2021 -0600

    qemu-sockets: fix unix socket path copy (again)
    
    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@xxxxxxxxxx>
    Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
    Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
    CC: qemu-stable@xxxxxxxxxx
    (cherry picked from commit 118d527f2e4baec5fe8060b22a6212468b8e4d3f)
    Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
---
 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 */
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#staging



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.