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 307b2887c44e5f22d471bdbc4a8451bdbe43ccd6..201ede465872e00e1f483299c3da5fbad0a22d1f 100644 (file)
--- 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 1069a69d2358fd4e055289251d3d3992bbb629f5..51997da70d5aed7cac75dc13dc0b7c6a4ed44437 100644 (file)
--- 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 af1a3347095994d8f2be3cb7dfdec40c13d4f9f3..279a0eb95817d60fe4a8a45576f76d1655a19f8e 100644 (file)
--- 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);