sockets: avoid leak of listen file descriptor

If we iterate over the full port range without successfully binding+listening on the socket, we'll try the next address, whereupon we overwrite the slisten file descriptor variable without closing it.

Rather than having two places where we open + close socket FDs on different iterations of nested for loops, re-arrange the code to always open+close within the same loop iteration.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 util/qemu-sockets.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Quoted 8 lines · Daniel P. Berrange

If we iterate over the full port range without successfully binding+listening on the socket, we'll try the next address, whereupon we overwrite the slisten file descriptor variable without closing it.

Rather than having two places where we open + close socket FDs on different iterations of nested for loops, re-arrange the code to always open+close within the same loop iteration.

Quoted 4 lines

Signed-off-by Daniel P. Berrange <[email protected]>

---
 util/qemu-sockets.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Self-NACK this has a bug too

Regards,
Daniel

Signature · 3 lines

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

If we iterate over the full port range without successfully binding+listening on the socket, we'll try the next address, whereupon we overwrite the slisten file descriptor variable without closing it.

Rather than having two places where we open + close socket FDs on different iterations of nested for loops, re-arrange the code to always open+close within the same loop iteration.

Signed-off-by Daniel P. Berrange <[email protected]>

---
util/qemu-sockets.c | 31 +++++++++++++++++++------------

1 file changed, 19 insertions(+), 12 deletions(-)

util/qemu-sockets.c +1 -1

index b47fb45885..a319338cca 100644
@@ -207,7 +207,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
    char uaddr[INET6_ADDRSTRLEN+1];
    char uport[33];
    int rc, port_min, port_max, p;
-    int slisten = 0;
+    int slisten = -1;
    int saved_errno = 0;
    bool socket_created = false;
    Error *err = NULL;

util/qemu-sockets.c +17 -0

        port_min = inet_getport(e);
        port_max = saddr->has_to ? saddr->to + port_offset : port_min;
        for (p = port_min; p <= port_max; p++) {
            inet_setport(e, p);
+
+            slisten = create_fast_reuse_socket(e);
+            if (slisten < 0) {
+                /* First time we expect we might fail to create the socket
+                 * eg if 'e' has AF_INET6 but ipv6 kmod is not loaded.
+                 * Later iterations should always succeeed if first iteration
+                 * worked though, so treat that as fatal.
+                 */
+                if (p == port_min) {
+                    continue;
+                } else {
+                    error_setg_errno(errp, errno,
+                                     "Failed to recreate failed listening socket");
+                    goto listen_failed;
+                }
+            }
+
            rc = try_bind(slisten, saddr, e);
            if (rc) {
                if (errno == EADDRINUSE) {

@@ -299,12 +311,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
             * socket to allow bind attempts for subsequent ports:
             */
            closesocket(slisten);