From: Willy Tarreau <w@1wt.eu>
Date: Fri, 1 May 2020 11:15:32 +0000 (+0200)
Subject: BUG/MEDIUM: shctx: bound the number of loops that can happen around the lock
X-Git-Tag: v2.2-dev7~32
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=86c6a9221aa5bd7516300f399e87aba09d61de06;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: shctx: bound the number of loops that can happen around the lock

Given that a "count" value of 32M was seen in _shctx_wait4lock(), it
is very important to prevent this from happening again. It's absolutely
essential to prevent the value from growing unbounded because with an
increase of the number of threads, the number of successive failed
attempts will necessarily grow.

Instead now we're scanning all 2^p-1 values from 3 to 255 and are
bounding to count to 255 so that in the worst case each thread tries an
xchg every 255 failed read attempts. That's one every 4 on average per
thread when there are 64 threads, which corresponds to the initial count
of 4 for the first attempt so it seems like a reasonable value to keep a
low latency.

The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.
---

diff --git a/include/proto/shctx.h b/include/proto/shctx.h
index ea62a12960..25bf377f23 100644
--- a/include/proto/shctx.h
+++ b/include/proto/shctx.h
@@ -96,7 +96,7 @@ static inline void _shctx_wait4lock(unsigned int *count, unsigned int *uaddr, in
 		if (*uaddr != value)
 			return;
         }
-        *count = *count << 1;
+        *count = (unsigned char)((*count << 1) + 1);
 }
 
 #define _shctx_awakelocker(a)
@@ -156,7 +156,7 @@ static inline unsigned char atomic_dec(unsigned int *ptr)
 static inline void _shctx_lock(struct shared_context *shctx)
 {
 	unsigned int x;
-	unsigned int count = 4;
+	unsigned int count = 3;
 
 	x = cmpxchg(&shctx->waiters, 0, 1);
 	if (x) {