From: Florian Forster <octo@huhu.verplant.org>
Date: Wed, 30 Sep 2009 20:49:16 +0000 (+0200)
Subject: unixsock plugin: Fix a (well hidden) race condition.
X-Git-Tag: collectd-4.7.4~3
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=5b5fff5bbdc5515d7f4c2a01df47373f4ac44847;p=collectd.git

unixsock plugin: Fix a (well hidden) race condition.

Within the client handling thread, fdopen is called twice on the file
descriptor passed to the thread. Later those file handles are closed like:

  fclose (fhin);
  fclose (fhout);

This is a race condition, because the first call to fclose will close the file
descriptor. The second call to fclose will try the same. Usually, it would fail
silently and all is well. On a busy machine, however, another thread may just
have opened a file or accepted a socket. In that case an arbitrary file
descriptor is closed. If the file descriptor is opened yet again fast enough,
data may even end up in a totally wrong location.

As a work-around the file descriptor is not dup'ed so each fdopen operates on
its own file descriptor. As an alternative the "r+" mode and a single file
handle may be suitable, too.

Many thanks to Sven Trenkel for pointing me into the right directioin :)
---

diff --git a/src/unixsock.c b/src/unixsock.c
index 57f34501..38f9025e 100644
--- a/src/unixsock.c
+++ b/src/unixsock.c
@@ -157,32 +157,45 @@ static int us_open_socket (void)
 
 static void *us_handle_client (void *arg)
 {
-	int fd;
+	int fdin;
+	int fdout;
 	FILE *fhin, *fhout;
 
-	fd = *((int *) arg);
+	fdin = *((int *) arg);
 	free (arg);
 	arg = NULL;
 
-	DEBUG ("unixsock plugin: us_handle_client: Reading from fd #%i", fd);
+	DEBUG ("unixsock plugin: us_handle_client: Reading from fd #%i", fdin);
 
-	fhin  = fdopen (fd, "r");
+	fdout = dup (fdin);
+	if (fdout < 0)
+	{
+		char errbuf[1024];
+		ERROR ("unixsock plugin: dup failed: %s",
+				sstrerror (errno, errbuf, sizeof (errbuf)));
+		close (fdin);
+		pthread_exit ((void *) 1);
+	}
+
+	fhin  = fdopen (fdin, "r");
 	if (fhin == NULL)
 	{
 		char errbuf[1024];
 		ERROR ("unixsock plugin: fdopen failed: %s",
 				sstrerror (errno, errbuf, sizeof (errbuf)));
-		close (fd);
+		close (fdin);
+		close (fdout);
 		pthread_exit ((void *) 1);
 	}
 
-	fhout = fdopen (fd, "w");
+	fhout = fdopen (fdout, "w");
 	if (fhout == NULL)
 	{
 		char errbuf[1024];
 		ERROR ("unixsock plugin: fdopen failed: %s",
 				sstrerror (errno, errbuf, sizeof (errbuf)));
-		fclose (fhin); /* this closes fd as well */
+		fclose (fhin); /* this closes fdin as well */
+		close (fdout);
 		pthread_exit ((void *) 1);
 	}
 
@@ -230,11 +243,12 @@ static void *us_handle_client (void *arg)
 
 		fields_num = strsplit (buffer_copy, fields,
 				sizeof (fields) / sizeof (fields[0]));
-
 		if (fields_num < 1)
 		{
-			close (fd);
-			break;
+			fprintf (fhout, "-1 Internal error\n");
+			fclose (fhin);
+			fclose (fhout);
+			pthread_exit ((void *) 1);
 		}
 
 		if (strcasecmp (fields[0], "getval") == 0)