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.
This commit is contained in:
Willy Tarreau 2024-03-26 15:12:19 +01:00
parent bd98db5078
commit 40d1c84bf0
2 changed files with 4 additions and 6 deletions

View File

@ -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 */

View File

@ -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);