From: Willy Tarreau <w@1wt.eu>
Date: Tue, 19 May 2020 17:21:45 +0000 (+0200)
Subject: BUG/MEDIUM: ring: write-lock the ring while attaching/detaching
X-Git-Tag: v2.2-dev8~33
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=223ddedb467494d78ebf67e0ac19206f767ffe6a;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: ring: write-lock the ring while attaching/detaching

The LIST_ADDQ() and LIST_DEL_INIT() calls made to attach/detach a waiter
to the ring were made under a read lock which was sufficient in front of
the writer's write lock. But it's not sufficient against other readers!
Thus theorically multiple "show events" on the same ring buffer on the
CLI could result in a crash, even though for now I couldn't manage to
reproduce it.

This fixes commit 1d181e489c ("MEDIUM: ring: implement a wait mode for
watchers") so it must be backported to 2.1, possibly further if the ring
code gets backported.
---

diff --git a/src/ring.c b/src/ring.c
index b94b5699bf..ec0656fbcf 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -248,9 +248,11 @@ int cli_io_handler_show_ring(struct appctx *appctx)
 	if (unlikely(si_ic(si)->flags & (CF_WRITE_ERROR|CF_SHUTW)))
 		return 1;
 
-	HA_RWLOCK_RDLOCK(LOGSRV_LOCK, &ring->lock);
-
+	HA_RWLOCK_WRLOCK(LOGSRV_LOCK, &ring->lock);
 	LIST_DEL_INIT(&appctx->ctx.cli.l0);
+	HA_RWLOCK_WRUNLOCK(LOGSRV_LOCK, &ring->lock);
+
+	HA_RWLOCK_RDLOCK(LOGSRV_LOCK, &ring->lock);
 
 	/* explanation for the initialization below: it would be better to do
 	 * this in the parsing function but this would occasionally result in
@@ -321,7 +323,9 @@ int cli_io_handler_show_ring(struct appctx *appctx)
 		 */
 		if (!si_oc(si)->output && !(si_oc(si)->flags & CF_SHUTW)) {
 			/* let's be woken up once new data arrive */
+			HA_RWLOCK_WRLOCK(LOGSRV_LOCK, &ring->lock);
 			LIST_ADDQ(&ring->waiters, &appctx->ctx.cli.l0);
+			HA_RWLOCK_WRUNLOCK(LOGSRV_LOCK, &ring->lock);
 			si_rx_endp_done(si);
 			ret = 0;
 		}
@@ -340,7 +344,7 @@ void cli_io_release_show_ring(struct appctx *appctx)
 	if (!ring)
 		return;
 
-	HA_RWLOCK_RDLOCK(LOGSRV_LOCK, &ring->lock);
+	HA_RWLOCK_WRLOCK(LOGSRV_LOCK, &ring->lock);
 	if (ofs != ~0) {
 		/* reader was still attached */
 		ofs -= ring->ofs;
@@ -349,7 +353,7 @@ void cli_io_release_show_ring(struct appctx *appctx)
 		HA_ATOMIC_SUB(b_peek(&ring->buf, ofs), 1);
 	}
 	HA_ATOMIC_SUB(&ring->readers_count, 1);
-	HA_RWLOCK_RDUNLOCK(LOGSRV_LOCK, &ring->lock);
+	HA_RWLOCK_WRUNLOCK(LOGSRV_LOCK, &ring->lock);
 }