From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Tue, 21 May 2024 09:13:46 +0000 (+0200)
Subject: BUG/MINOR: ring: free ring's allocated area not ring's usable area when using maps
X-Git-Tag: v3.0-dev13~64
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0cfbeb1ae831e3cc9392ea242717ffbc0ff37e9f;p=thirdparty%2Fhaproxy.git

BUG/MINOR: ring: free ring's allocated area not ring's usable area when using maps

Since 40d1c84bf0 ("BUG/MAJOR: ring: free the ring storage not the ring
itself when using maps"), munmap() call for startup_logs's ring and
file-backed rings fails to work (EINVAL) and causes memory leaks during
process cleanup.

munmap() fails because it is called with the ring's usable area pointer
which is an offset from the underlying original memory block allocated
using mmap(). Indeed, ring_area() helper function was misused because
it didn't explicitly mention that the returned address corresponds to
the usable storage's area, not the allocated one.

To fix the issue, we add an explicit ring_allocated_area() helper to
return the allocated area for the ring, just like we already have
ring_allocated_size() for the allocated size, and we properly use both
the allocated size and allocated area to manipulate them using munmap()
and msync().

No backport needed.
---

diff --git a/include/haproxy/ring.h b/include/haproxy/ring.h
index 307b2887c4..201ede4658 100644
--- a/include/haproxy/ring.h
+++ b/include/haproxy/ring.h
@@ -45,12 +45,20 @@ size_t ring_max_payload(const struct ring *ring);
 int ring_dispatch_messages(struct ring *ring, void *ctx, size_t *ofs_ptr, size_t *last_ofs_ptr, uint flags,
 			   ssize_t (*msg_handler)(void *ctx, struct ist v1, struct ist v2, size_t ofs, size_t len));
 
-/* returns the ring storage's area */
+/* returns the ring storage's usable area */
 static inline void *ring_area(const struct ring *ring)
 {
 	return ring->storage->area;
 }
 
+/* returns the allocated area for the ring. It covers the whole
+ * area made of both the ring_storage and the usable area.
+ */
+static inline void *ring_allocated_area(const struct ring *ring)
+{
+	return ring->storage;
+}
+
 /* returns the number of bytes in the ring */
 static inline size_t ring_data(const struct ring *ring)
 {
diff --git a/src/errors.c b/src/errors.c
index 1069a69d23..51997da70d 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -190,7 +190,7 @@ void startup_logs_free(struct ring *r)
 {
 #ifdef USE_SHM_OPEN
 	if (r == shm_startup_logs)
-		munmap(ring_area(r), STARTUP_LOG_SIZE);
+		munmap(ring_allocated_area(r), STARTUP_LOG_SIZE);
 #endif /* ! USE_SHM_OPEN */
 	ring_free(r);
 }
diff --git a/src/sink.c b/src/sink.c
index af1a334709..279a0eb958 100644
--- a/src/sink.c
+++ b/src/sink.c
@@ -695,8 +695,8 @@ static void sink_free(struct sink *sink)
 		return;
 	if (sink->type == SINK_TYPE_BUFFER) {
 		if (sink->store) {
-			size_t size = (ring_size(sink->ctx.ring) + 4095UL) & -4096UL;
-			void *area = ring_area(sink->ctx.ring);
+			size_t size = (ring_allocated_size(sink->ctx.ring) + 4095UL) & -4096UL;
+			void *area = ring_allocated_area(sink->ctx.ring);
 
 			msync(area, size, MS_SYNC);
 			munmap(area, size);