socket: clear port when unable to generate local port

When running out of local ports, _nl_socket_generate_local_port_no_release()
would leave the socket with port UINT32_MAX. That means if nl_connect()
fails due to out-of-ports, it would leave the port id assigned to an
invalid port and the socket instance was not re-usable until the user
called nl_socket_set_local_port(). Fix that by resetting the local port
to zero.

Thereby, also change generate_local_port() to return zero when
running out of ports. zero is a more natural value for ~no port found~.
It also matches the port that _nl_socket_generate_local_port_no_release()
uses when failing to generate a port.

Also ensure that zero cannot be returned as valid port by generate_local_port().
Arguably, that would only be possible if (getpid() & 0x3FFFFF)
returns zero. Just be extra cautious.

Signed-off-by: Thomas Haller <thaller@redhat.com>

1 files changed, 22 insertions, 8 deletions

diff --git a/lib/socket.c b/lib/socket.c
index b29d1da..a1e0873 100644
--- a/lib/socket.c
+++ b/lib/socket.c
@@ -108,7 +108,9 @@ static uint32_t generate_local_port(void)
 
 			nl_write_unlock(&port_map_lock);
 
-			return pid + (((uint32_t)n) << 22);
+			/* ensure we don't return zero. */
+			pid = pid + (((uint32_t)n) << 22);
+			return pid ? pid : 1024;
 		}
 	}
 
@@ -116,7 +118,7 @@ static uint32_t generate_local_port(void)
 
 	/* Out of sockets in our own PID namespace, what to do? FIXME */
 	NL_DBG(1, "Warning: Ran out of unique local port namespace\n");
-	return UINT32_MAX;
+	return 0;
 }
 
 static void release_local_port(uint32_t port)
@@ -124,9 +126,6 @@ static void release_local_port(uint32_t port)
 	int nr;
 	uint32_t mask;
 
-	if (port == UINT32_MAX)
-		return;
-
 	BUG_ON(port == 0);
 
 	nr = port >> 22;
@@ -167,7 +166,7 @@ void _nl_socket_used_ports_set(uint32_t *used_ports, uint32_t port)
 	nr /= 32;
 
 	/*
-	BUG_ON(port == UINT32_MAX || port == 0 || (getpid() & 0x3FFFFF) != (port & 0x3FFFFF));
+	BUG_ON(port == 0 || (getpid() & 0x3FFFFF) != (port & 0x3FFFFF));
 	BUG_ON(used_ports[nr] & mask);
 	*/
 
@@ -345,8 +344,13 @@ uint32_t _nl_socket_generate_local_port_no_release(struct nl_sock *sk)
 	 * the previously generated port. */
 
 	port = generate_local_port();
-	sk->s_flags &= ~NL_OWN_PORT;
 	sk->s_local.nl_pid = port;
+	if (port == 0) {
+		/* failed to find an unsed port. Restore the socket to have an
+		 * unspecified port. */
+		sk->s_flags |= NL_OWN_PORT;
+	} else
+		sk->s_flags &= ~NL_OWN_PORT;
 	return port;
 }
 /** \endcond */
@@ -359,6 +363,8 @@ uint32_t _nl_socket_generate_local_port_no_release(struct nl_sock *sk)
 uint32_t nl_socket_get_local_port(const struct nl_sock *sk)
 {
 	if (sk->s_local.nl_pid == 0) {
+		struct nl_sock *sk_mutable = (struct nl_sock *) sk;
+
 		/* modify the const argument sk. This is justified, because
 		 * nobody ever saw the local_port from externally. So, we
 		 * initilize it on first use.
@@ -368,7 +374,15 @@ uint32_t nl_socket_get_local_port(const struct nl_sock *sk)
 		 * is not automatically threadsafe anyway, so the user is not
 		 * allowed to do that.
 		 */
-		return _nl_socket_generate_local_port_no_release((struct nl_sock *) sk);
+		sk_mutable->s_local.nl_pid = generate_local_port();
+		if (sk_mutable->s_local.nl_pid == 0) {
+			/* could not generate a local port. Assign UINT32_MAX to preserve
+			 * backward compatibility. A user who cares can clear that anyway
+			 * with nl_socket_set_local_port(). */
+			sk_mutable->s_local.nl_pid = UINT32_MAX;
+			sk_mutable->s_flags |= NL_OWN_PORT;
+		} else
+			sk_mutable->s_flags &= ~NL_OWN_PORT;
 	}
 	return sk->s_local.nl_pid;
 }