BUG/MAJOR: ring: free the ring storage not the ring itself when using maps

A recent issue was uncovered by the CI which started to randomly report
segfaults on a few tests, and more systematically on FreeBSD. It turn
out that it was introduced by recent commit 03816ccfa9 ("MAJOR: ring:
insert an intermediary ring_storage level"), which overlooked the munmap()
path of the sink and startup logs: once the ring and its storage were
split, it was no longer correct to munmap() the ring, only its storage
area needs to be unmapped, and the ring must always be freed separately.

Thanks to Christopher and William for their help at trying to reproduce
it and figure the circumstances that triggers it.

No backport is needed.

diff --git a/src/errors.c b/src/errors.c
index 7a874baf7e3d14d6170476f1456196badf784f8a..62a5ebd3b913613f44b8a124f4e6dbbce486fa0d 100644 (file)
--- a/src/errors.c
+++ b/src/errors.c
@@ -190,10 +190,9 @@ void startup_logs_free(struct ring *r)
 {
 #ifdef USE_SHM_OPEN
        if (r == shm_startup_logs)
-               munmap(r, STARTUP_LOG_SIZE);
-       else
+               munmap(ring_area(r), STARTUP_LOG_SIZE);
 #endif /* ! USE_SHM_OPEN */
-               ring_free(r);
+       ring_free(r);
 }
 
 /* duplicate a startup logs which was previously allocated in a shm */

diff --git a/src/sink.c b/src/sink.c
index 1195c359c93cd0681d9149cfae358d5013cc24df..76f3ffb147bf075e33db7a7634cb18a5204f1af0 100644 (file)
--- a/src/sink.c
+++ b/src/sink.c
@@ -696,14 +696,13 @@ static void sink_free(struct sink *sink)
        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) - sizeof(*sink->ctx.ring));
+                       void *area = ring_area(sink->ctx.ring);
 
                        msync(area, size, MS_SYNC);
                        munmap(area, size);
                        ha_free(&sink->store);
                }
-               else
-                       ring_free(sink->ctx.ring);
+               ring_free(sink->ctx.ring);
        }
        LIST_DEL_INIT(&sink->sink_list); // remove from parent list
        task_destroy(sink->forward_task);