Bug 677527 - port allocation/deallocation needs to be made thread-safe
Summary: port allocation/deallocation needs to be made thread-safe
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: libnl
Version: 14
Hardware: Unspecified
OS: Linux
high
urgent
Target Milestone: ---
Assignee: Dan Williams
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-15 04:54 UTC by Stefan Berger
Modified: 2011-05-09 03:54 UTC (History)
3 users (show)

Fixed In Version: libnl-1.1-14.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 677724 (view as bug list)
Environment:
Last Closed: 2011-04-12 21:24:14 UTC


Attachments (Terms of Use)
make libnl thread safe and fix a port allocation bug (1.37 KB, patch)
2011-02-15 13:42 UTC, Stefan Berger
no flags Details | Diff
patch against libnl-1.1-12 (1.87 KB, patch)
2011-02-25 11:45 UTC, Stefan Berger
no flags Details | Diff

Description Stefan Berger 2011-02-15 04:54:59 UTC
Description of problem:

The port allocation/deallocation in socket.c needs to be made thread-safe. Too many different callers hidden in libraries (netcf) may call this code at the same time and corrupt the port allocation. Concrete problem may arise in libvirt.

Since I am a nice guy I provide you a patch, which I haven't tested, though:


Index: lib/socket.c
===================================================================
--- lib.orig/socket.c
+++ lib/socket.c
@@ -89,6 +89,8 @@
  * @{
  */
 
+#include <pthread.h>
+
 #include <netlink-local.h>
 #include <netlink/netlink.h>
 #include <netlink/utils.h>
@@ -117,12 +119,15 @@ static void __init init_default_cb(void)
 }
 
 static uint32_t used_ports_map[32];
+static pthread_mutex_t port_map_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static uint32_t generate_local_port(void)
 {
 	int i, n;
 	uint32_t pid = getpid() & 0x3FFFFF;
 
+	pthread_mutex_lock(&port_map_mutex);
+
 	for (i = 0; i < 32; i++) {
 		if (used_ports_map[i] == 0xFFFFFFFF)
 			continue;
@@ -141,6 +146,8 @@ static uint32_t generate_local_port(void
 		}
 	}
 
+	pthread_mutex_unlock(&port_map_mutex);
+
 	/* Out of sockets in our own PID namespace, what to do? FIXME */
 	return UINT_MAX;
 }
@@ -152,8 +159,12 @@ static void release_local_port(uint32_t 
 	if (port == UINT_MAX)
 		return;
 	
+
 	nr = port >> 22;
-	used_ports_map[nr / 32] &= ~((nr % 32) + 1);
+
+	pthread_mutex_lock(&port_map_mutex);
+	used_ports_map[nr / 32] &= ~(1 << (nr & 0x1f));
+	pthread_mutex_unlock(&port_map_mutex);
 }
 
 /**

Now need to link against -lpthread as well.



Version-Release number of selected component (if applicable):

libnl-1.1-12.fc14.x86_64
libnl-debuginfo-1.1-12.fc14.x86_64
libnl-devel-1.1-12.fc14.x86_64

Comment 1 Stefan Berger 2011-02-15 13:42:50 UTC
Created attachment 478874 [details]
make libnl thread safe and fix a port allocation bug

Here's the fix that I built libnl with. I did not link with -lpthread, so you may want to add this.

Index: libnl-1.1/lib/socket.c
===================================================================
--- libnl-1.1/lib.orig/socket.c
+++ libnl-1.1/lib/socket.c
@@ -89,6 +89,8 @@
  * @{
  */
 
+#include <pthread.h>
+
 #include <netlink-local.h>
 #include <netlink/netlink.h>
 #include <netlink/utils.h>
@@ -117,12 +119,15 @@ static void __init init_default_cb(void)
 }
 
 static uint32_t used_ports_map[32];
+static pthread_mutex_t port_map_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static uint32_t generate_local_port(void)
 {
 	int i, n;
 	uint32_t pid = getpid() & 0x3FFFFF;
 
+	pthread_mutex_lock(&port_map_mutex);
+
 	for (i = 0; i < 32; i++) {
 		if (used_ports_map[i] == 0xFFFFFFFF)
 			continue;
@@ -136,11 +141,15 @@ static uint32_t generate_local_port(void
 
 			/* PID_MAX_LIMIT is currently at 2^22, leaving 10 bit
 			 * to, i.e. 1024 unique ports per application. */
-			return pid + (n << 22);
 
+			pthread_mutex_unlock(&port_map_mutex);
+
+			return pid + (n << 22);
 		}
 	}
 
+	pthread_mutex_unlock(&port_map_mutex);
+
 	/* Out of sockets in our own PID namespace, what to do? FIXME */
 	return UINT_MAX;
 }
@@ -153,7 +162,10 @@ static void release_local_port(uint32_t 
 		return;
 	
 	nr = port >> 22;
-	used_ports_map[nr / 32] &= ~((nr % 32) + 1);
+	
+	pthread_mutex_lock(&port_map_mutex);
+	used_ports_map[nr / 32] &= ~(1 << (nr & 0x1f));
+	pthread_mutex_unlock(&port_map_mutex);
 }
 
 /**

Comment 2 Stefan Berger 2011-02-25 11:45:55 UTC
Created attachment 480973 [details]
patch against libnl-1.1-12

I added a hunk so that the final library also gets linked with -lpthread. This is necessary due to the mutex being used to protect the global used_ports_map.

Comment 3 Dan Williams 2011-03-21 19:32:30 UTC
Thomas, what do you think?  What sort of solution would you apply upstream?  I can ship this patch in the mean time.

Comment 4 Fedora Update System 2011-03-21 19:48:35 UTC
libnl-1.1-14.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/libnl-1.1-14.fc15

Comment 5 Fedora Update System 2011-03-21 19:56:10 UTC
libnl-1.1-14.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/libnl-1.1-14.fc14

Comment 6 Fedora Update System 2011-03-21 20:09:07 UTC
libnl-1.1-14.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/libnl-1.1-14.fc13

Comment 7 Thomas Graf 2011-03-21 21:54:45 UTC
I can't think of any better alternatives. I will merge this upstream.

Comment 8 Fedora Update System 2011-04-12 21:24:05 UTC
libnl-1.1-14.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 9 Fedora Update System 2011-04-12 21:31:47 UTC
libnl-1.1-14.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2011-05-09 03:54:42 UTC
libnl-1.1-14.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.