Skip to content

Commit

Permalink
BUG/MAJOR: ring: free the ring storage not the ring itself when using…
Browse files Browse the repository at this point in the history
… 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 03816cc ("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.
  • Loading branch information
wtarreau committed Mar 26, 2024
1 parent bd98db5 commit 40d1c84
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 6 deletions.
5 changes: 2 additions & 3 deletions src/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
5 changes: 2 additions & 3 deletions src/sink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 40d1c84

Please sign in to comment.