From: Willy Tarreau <w@1wt.eu>
Date: Thu, 26 Jul 2018 05:33:44 +0000 (+0200)
Subject: MINOR: queue: make sure pendconn->strm->pend_pos is always valid
X-Git-Tag: v1.9-dev1~23
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1a60d6218af005f847da4ade4465012d439a7cd;p=thirdparty%2Fhaproxy.git

MINOR: queue: make sure pendconn->strm->pend_pos is always valid

pendconn_add() used to assign strm->pend_pos very late, after unlocking
the queue, so that a watching thread could see a random value in
pendconn->strm->pend_pos even while holding the lock on the element and
the queue itself. While there's currently nothing wrong with this, it
costs nothing to arrange it and will simplify code analysis later.
---

diff --git a/src/queue.c b/src/queue.c
index d522f0e91a..2773826402 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -71,9 +71,6 @@
  *     queue to be locked/unlocked.
  *
  *   - a pendconn doesn't switch between queues, it stays where it is.
- *
- *   - strm->pend_pos is assigned late so pendconn->strm->pend_pos could be met
- *     uninitialized by another thread and must not be relied on.
  */
 
 #include <common/config.h>
@@ -318,6 +315,7 @@ struct pendconn *pendconn_add(struct stream *strm)
 		if (srv->nbpend > srv->counters.nbpend_max)
 			srv->counters.nbpend_max = srv->nbpend;
 		LIST_ADDQ(&srv->pendconns, &p->list);
+		strm->pend_pos = p;
 		HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
 	}
 	else {
@@ -327,10 +325,10 @@ struct pendconn *pendconn_add(struct stream *strm)
 		if (px->nbpend > px->be_counters.nbpend_max)
 			px->be_counters.nbpend_max = px->nbpend;
 		LIST_ADDQ(&px->pendconns, &p->list);
+		strm->pend_pos = p;
 		HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
 	}
 	HA_ATOMIC_ADD(&px->totpend, 1);
-	strm->pend_pos = p;
 	return p;
 }