`ring_buffer`: iovec helpers under-report free/used space by one byte in wrap-around states

[eriksjolund]

Claude Opus 4.8 detected a small problem with the ring_buffer.

click me to show claude output

ring_buffer: iovec helpers under-report free/used space by one byte in wrap-around states

Summary

In src/libcrun/ring_buffer.c, ring_buffer_get_read_iov() and
ring_buffer_get_write_iov() compute the reserved "sentinel" byte as if it
always lived at the physical end of the buffer. It does not — logically the
sentinel sits at index head - 1. As a result, in every wrap-around state
(head != 0) both helpers expose one byte fewer than
ring_buffer_get_space_available() / ring_buffer_get_data_available() report.

This does not corrupt data and is not a security issue (see "Impact"
below), but the reported free/used space no longer matches what a single
readv() / writev() can actually transfer, and the buffer wastes one byte of
capacity whenever it is wrapped.

Details

The buffer uses the standard "one reserved slot" scheme: ring_buffer_make()
allocates size + 1 bytes so that head == tail means empty and
(tail + 1) % size == head means full. That part is correct.

The problem is in the wrap-around (else) branch of the two iovec helpers:

/* ring_buffer_get_read_iov(), head > tail */
iov[iov_count].iov_len = rb->size - rb->head - 1; /* Don't read from reserved byte */

/* ring_buffer_get_write_iov(), tail >= head */
iov[iov_count].iov_len = rb->size - rb->tail - 1; /* Don't write to reserved byte */

The - 1 assumes the reserved slot is the last physical byte (size - 1). But
the reserved slot is the single free slot immediately before head, i.e. index
head - 1. Those two only coincide when head == 0. For any head != 0:

• the read side skips the last physical byte even though it may hold data, and
• the write side refuses the last physical byte even though it is free,

so each helper returns a total that is one less than the advertised
space/data.

Reproduction (white-box)

For any wrap-around state the sum of the iovec lengths disagrees with the
advertised counts. Example for a buffer created with ring_buffer_make(8)
(internal size 9):

head	tail	data_available	space_available	read iov total	write iov total
1	1	0	8	0	7
1	2	1	7	1	6
2	0	7	1	6	1
1	8	7	1	7	0

The last row is the sharpest case: space_available reports 1, but a single
ring_buffer_read() transfers 0 bytes and returns EAGAIN — a stall while
space is still advertised.

The existing unit test (test_ring_buffer_read_write) does not catch this
because it fully drains the buffer every cycle, and ring_buffer_write() resets
head/tail to 0 whenever the buffer empties — so head is always 0 when
the accounting is checked, i.e. the wrap-around states are never exercised.

Impact

• No data corruption. The implementation is internally self-consistent: it
  never writes the last physical byte, so it never needs to read it either.
  A round-trip stress test kept in wrap-around states (100k+ bytes) passes
  clean, including under ASan/UBSan.
• Capacity is under-utilized by one byte whenever the buffer is wrapped.
• ring_buffer_get_space_available() over-reports the space usable in a
  single readv() by one byte in wrap-around states.

The only in-tree consumer is channel_fd_pair_process() in utils.c, which
uses the accessors only as > 0 guards before calling read/write. There the
practical effect is minor — an occasional no-op readv() and/or a spurious
EPOLLIN wakeup — but a caller that trusts get_space_available() to mean
"I can buffer N bytes right now" would be off by one, and the head == 1, tail == size - 1 state produces a real (if narrow) read stall while space is
advertised.

Suggested fix

Reserve the sentinel at its true position (head - 1), which is the physical
end of the buffer only when head == 0:

/* read side: data is never stored in the reserved slot, so read to the end */
iov[iov_count].iov_len = rb->size - rb->head;

/* write side: reserve the last byte only when the sentinel is actually there */
iov[iov_count].iov_len = rb->size - rb->tail - (rb->head == 0 ? 1 : 0);

With this change the invariant write_iov_total == space_available and
read_iov_total == data_available holds for every (head, tail) combination,
the wasted byte is reclaimed, and the round-trip/ASan/UBSan tests still pass.

I have a patch ready that includes this fix plus a white-box regression test
(iterating every (head, tail) for several sizes and asserting the invariant);
happy to open a PR if this looks right.

Environment

• Component: src/libcrun/ring_buffer.c (ring_buffer_get_read_iov,
  ring_buffer_get_write_iov)
• Affects: current main

I also asked Claude for a minimal reproducer:

click me to show claude output

---

Drop this in src/libcrun/, then
gcc -DHAVE_CONFIG_H -I../.. -I../../libocispec/src -I/usr/include/json-c repro.c -o repro:

#include "ring_buffer.c"
#include <stdio.h>

int libcrun_make_error (libcrun_error_t *e, int s, const char *m, ...) { return -1; }

int
main (void)
{
  struct ring_buffer *rb = ring_buffer_make (8);
  struct iovec iov[2];
  size_t w = 0;
  int i, n;

  rb->head = 1;
  rb->tail = 1;

  n = ring_buffer_get_write_iov (rb, iov);
  for (i = 0; i < n; i++)
    w += iov[i].iov_len;

  printf ("space_available=%zu  writable_via_iov=%zu\n",
          ring_buffer_get_space_available (rb), w);
  return 0;
}

Output:

space_available=8  writable_via_iov=7

[eriksjolund]

Let's close this issue one as I opened a new one

• ring_buffer: iovec helpers under-report free/used space by one byte in wrap-around states #2191

[eriksjolund]

I asked Claude Opus 5 (high)

Look for the most interesting bugs you can find in crun overall?

and to get the answer in Markdown format I added

Could you write your last answer again but now as a file in markdown format

Claude found this bug again (and two other unrelated bugs).

Claude suggested the same fix again.

Note, the bug analysis is a bit different though. Claude now writes about corrupted bytes.

In the previous analysis #2191 (comment)
Claude wrote No data corruption.

click me to show claude output related to the ring_buffer issue

Tree examined: 58c341a18d3eed62ecf74fd5523a25bd48397e63 (branch main, 2026-08-13, merge of PR #2183 / tag 1.29.1)

crun was built from this commit, so several of the findings below are confirmed at runtime rather than by reading alone.

---

1. Ring buffer silently corrupts the container's terminal stream

File: src/libcrun/ring_buffer.c

The most interesting one by far, because it's a data-integrity bug on a hot path with no error and no crash.

The buffer uses the standard "reserve one slot" full/empty scheme, but both iov builders reserve the last physical byte instead of the slot before head:

/* read side, wrapped case */
iov[iov_count].iov_len = rb->size - rb->head - 1;  /* Don't read from reserved byte */
/* write side */
iov[iov_count].iov_len = rb->size - rb->tail - 1;  /* Don't write to reserved byte */

So region 1 stops at index size-2 and region 2 restarts at index 0 — leaving a one-byte hole. But ring_buffer_advance_nocheck_head/tail then does (head + ret) % size, which assumes the two regions are contiguous. Any readv/writev that spans both regions therefore lands head/tail one position short of where the data actually is. Meanwhile ring_buffer_get_data_available() computes the correct count, so the accounting and the iovecs disagree by one.

Result: after the first wrap, a byte gets dropped and a stale byte emitted in its place.

Reproduction

Through the public API only (ring_buffer_read / ring_buffer_write) with real non-blocking pipes, at BUFSIZ — the exact size container.c:2058 uses for crun run -t terminal forwarding:

cap=8192 seed=1: STREAM CORRUPTED at byte 84715 (expected 0xeb, got 0xec)
cap=8192 seed=2: STREAM CORRUPTED at byte 94121 (expected 0xa9, got 0xaa)
cap=8192 seed=3: STREAM CORRUPTED at byte 79609 (expected 0xf9, got 0xfa)

It needs bursty traffic (transfers large enough to span both regions) plus a consumer that doesn't drain instantly — i.e. podman run -t piping a large build log into a slow terminal, roughly one corrupted byte per ~80 KB. A randomized model check over the two static iov functions fails within cap bytes on every seed tried.

Fix

Two lines, which pass both the model check and the pipe test:

/* read: the wrapped region really does run to the end of the buffer */
iov[iov_count].iov_len = rb->size - rb->head;
/* write: reserve only when head is at 0, since then the reserved slot IS size-1 */
iov[iov_count].iov_len = rb->size - rb->tail - (rb->head == 0 ? 1 : 0);

Latent stall in the same code

States like head == size-1, tail == 0 produce iov_count == 1 with iov_len == 0 while get_data_available() reports 1. channel_fd_pair_process then arms level-triggered EPOLLOUT on a writable fd forever — a 100% CPU spin. The fix above removes those states.

click me to show some more claude output

Worth reporting upstream

The ring buffer one especially — it's a correctness bug in stdout/stderr forwarding that would be very hard to diagnose from the symptom (occasional single-byte garbling in long terminal output, blamed on the terminal or the shell).

None of these look like security issues: the ring buffer stays in bounds, and the NULL deref is a local crash triggered by a config the invoking user already controls.