BUG/MEDIUM: ring: invert the length check to avoid an int overflow

Vincent Gramer reported in GH issue #3125 a case of crash on a BUG_ON()
condition in the rings. What happens is that a message that is one byte
less than the maximum ring size is emitted, and it passes all the checks,
but once inflated by the extra +1 for the refcount, it can no longer. But
the check was made based on message size compared to space left, except
that this space left can now be negative, which is a high positive for
size_t, so the check remained valid and triggered a BUG_ON() later.

Let's compute the size the other way around instead (i.e. current +
needed) since we can't have rings as large as half of the memory space
anyway, thus we have no risk of overflow on this one.

This needs to be backported to all versions supporting multi-threaded
rings (3.0 and above).

Thanks to Vincent for the easy and working reproducer.
This commit is contained in:
Willy Tarreau 2025-09-17 18:45:13 +02:00
parent 8c077c17eb
commit d53ad49ad1

View File

@ -320,7 +320,7 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const struct ist pfx[], siz
vp_ring_to_data(&v1, &v2, ring_area, ring_size, head_ofs, tail_ofs);
while (vp_size(v1, v2) > ring_size - needed - 1 - 1) {
while (vp_size(v1, v2) + needed + 1 + 1 > ring_size) {
/* we need to delete the oldest message (from the end),
* and we have to stop if there's a reader stuck there.
* Unless there's corruption in the buffer it's guaranteed
@ -339,7 +339,7 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const struct ist pfx[], siz
/* now let's update the buffer with the new tail if our message will fit */
new_tail_ofs = tail_ofs;
if (vp_size(v1, v2) <= ring_size - needed - 1 - 1) {
if (vp_size(v1, v2) + needed + 1 + 1 <= ring_size) {
vp_data_to_ring(v1, v2, ring_area, ring_size, &head_ofs, &tail_ofs);
/* update the new space in the buffer */