From: Emeric Brun <ebrun@haproxy.com>
Date: Tue, 7 Nov 2017 10:19:48 +0000 (+0100)
Subject: BUG/MEDIUM: splice/threads: pipe reuse list was not protected.
X-Git-Tag: v1.8-rc3~24
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d8b3b65;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: splice/threads: pipe reuse list was not protected.

The list is now protected using a global spinlock.
---

diff --git a/include/common/hathreads.h b/include/common/hathreads.h
index 460c2ea67f..3b8fb0bd84 100644
--- a/include/common/hathreads.h
+++ b/include/common/hathreads.h
@@ -174,6 +174,7 @@ enum lock_label {
 	DNS_LOCK,
 	PID_LIST_LOCK,
 	EMAIL_ALERTS_LOCK,
+	PIPES_LOCK,
 	LOCK_LABELS
 };
 struct lock_stat {
@@ -262,7 +263,8 @@ static inline void show_lock_stats()
 					   "UPDATED_SERVERS", "LBPRM", "SIGNALS", "STK_TABLE", "STK_SESS",
 					   "APPLETS", "PEER", "BUF_WQ", "STREAMS", "SSL", "SSL_GEN_CERTS",
 					   "PATREF", "PATEXP", "PATLRU", "VARS", "COMP_POOL", "LUA",
-					   "NOTIF", "SPOE_APPLET", "DNS", "PID_LIST", "EMAIL_ALERTS" };
+					   "NOTIF", "SPOE_APPLET", "DNS", "PID_LIST", "EMAIL_ALERTS",
+					   "PIPES" };
 	int lbl;
 
 	for (lbl = 0; lbl < LOCK_LABELS; lbl++) {
diff --git a/src/pipe.c b/src/pipe.c
index fce115e693..c7ce3c58dc 100644
--- a/src/pipe.c
+++ b/src/pipe.c
@@ -21,6 +21,7 @@
 
 struct pool_head *pool2_pipe = NULL;
 struct pipe *pipes_live = NULL; /* pipes which are still ready to use */
+HA_SPINLOCK_T pipes_lock;       /* lock used to protect pipes list */
 int pipes_used = 0;             /* # of pipes in use (2 fds each) */
 int pipes_free = 0;             /* # of pipes unused */
 
@@ -30,6 +31,7 @@ static void init_pipe()
 	pool2_pipe = create_pool("pipe", sizeof(struct pipe), MEM_F_SHARED);
 	pipes_used = 0;
 	pipes_free = 0;
+	HA_SPIN_INIT(&pipes_lock);
 }
 
 /* return a pre-allocated empty pipe. Try to allocate one if there isn't any
@@ -37,27 +39,28 @@ static void init_pipe()
  */
 struct pipe *get_pipe()
 {
-	struct pipe *ret;
+	struct pipe *ret = NULL;
 	int pipefd[2];
 
+	HA_SPIN_LOCK(PIPES_LOCK, &pipes_lock);
 	if (likely(pipes_live)) {
 		ret = pipes_live;
 		pipes_live = pipes_live->next;
 		pipes_free--;
 		pipes_used++;
-		return ret;
+		goto out;
 	}
 
 	if (pipes_used >= global.maxpipes)
-		return NULL;
+		goto out;
 
 	ret = pool_alloc2(pool2_pipe);
 	if (!ret)
-		return NULL;
+		goto out;
 
 	if (pipe(pipefd) < 0) {
 		pool_free2(pool2_pipe, ret);
-		return NULL;
+		goto out;
 	}
 #ifdef F_SETPIPE_SZ
 	if (global.tune.pipesize)
@@ -68,13 +71,12 @@ struct pipe *get_pipe()
 	ret->cons = pipefd[0];
 	ret->next = NULL;
 	pipes_used++;
+ out:
+	HA_SPIN_UNLOCK(PIPES_LOCK, &pipes_lock);
 	return ret;
 }
 
-/* destroy a pipe, possibly because an error was encountered on it. Its FDs
- * will be closed and it will not be reinjected into the live pool.
- */
-void kill_pipe(struct pipe *p)
+static void inline __kill_pipe(struct pipe *p)
 {
 	close(p->prod);
 	close(p->cons);
@@ -83,20 +85,34 @@ void kill_pipe(struct pipe *p)
 	return;
 }
 
+/* destroy a pipe, possibly because an error was encountered on it. Its FDs
+ * will be closed and it will not be reinjected into the live pool.
+ */
+void kill_pipe(struct pipe *p)
+{
+	HA_SPIN_LOCK(PIPES_LOCK, &pipes_lock);
+	__kill_pipe(p);
+	HA_SPIN_UNLOCK(PIPES_LOCK, &pipes_lock);
+	return;
+}
+
 /* put back a unused pipe into the live pool. If it still has data in it, it is
  * closed and not reinjected into the live pool. The caller is not allowed to
  * use it once released.
  */
 void put_pipe(struct pipe *p)
 {
+	HA_SPIN_LOCK(PIPES_LOCK, &pipes_lock);
 	if (p->data) {
-		kill_pipe(p);
-		return;
+		__kill_pipe(p);
+		goto out;
 	}
 	p->next = pipes_live;
 	pipes_live = p;
 	pipes_free++;
 	pipes_used--;
+ out:
+	HA_SPIN_UNLOCK(PIPES_LOCK, &pipes_lock);
 }