fix(streaming): count Streaming_PutSample capacity in samples, not bytes

[nicolas-rabault]

The bug

Streaming_PutSample guards its capacity with a comparison whose two sides are in different units:

LUOS_ASSERT((Streaming_GetAvailableSampleNB(stream) + size)
            <= ((uintptr_t)stream->end_ring_buffer - (uintptr_t)stream->ring_buffer));

Streaming_GetAvailableSampleNB() returns samples and size is samples, but end_ring_buffer - ring_buffer is a byte span. The guard is therefore too permissive by a factor of data_size, and the memcpy that follows can run past the end of the ring buffer.

Streaming_AddAvailableSampleNB already did the same check correctly, dividing that span by data_size — but Streaming_PutSample advances data_ptr itself and never calls it, so the correct check was never reached on that path.

This stayed hidden because every existing streaming test uses data_size == 1, where a sample count and a byte count are the same number.

Verified

Measured with a temporary probe on unpatched main, using a 16-sample float channel (64 bytes) with a 64-byte guard region placed immediately after it:

scenario	guard fires?	result
17 samples into a fresh 16-sample channel	no	accepted; 17 > 16 capacity
32 samples, data_ptr on the last slot	no	60 of 64 guard bytes clobbered — out-of-bounds write
20 samples into a fresh 16-sample channel	no	stays in bounds, but silently wraps: reports 4 samples available after writing 20

The 60-byte overrun is the decisive case. Arithmetic matches exactly: chunk1 = 64 - 60 = 4, chunk2 = 128 - 4 = 124, and 124 bytes written at ring_buffer in a 64-byte ring is 60 past the end. The first byte after the buffer held data[17], exactly as predicted.

Reachable from the network. Luos_ReceiveStreaming clamps to MAX_DATA_MSG_SIZE (128) and divides by data_size, yielding up to 32 samples on a 4-byte channel. Any channel with fewer than ~32 samples of capacity can be driven out of bounds by a remote node's message. That is precisely the second row above.

Refuted

One detail of the original report does not hold. Putting 20 samples into a fresh 16-sample float channel does not write past the end of the buffer. The wrap-around branch splits the copy: chunk1 = 64 fills the ring, then chunk2 = 16 wraps and lands at the start of the ring. The guard region was untouched (0 of 64 bytes).

What actually happens there is silent in-buffer data loss: 16 of the 20 samples are destroyed, and the channel afterwards reports 4 samples available. The out-of-bounds write needs data_ptr to be somewhere other than the start of the ring — which is the normal steady state of a running channel, so this is not an exotic condition.

The fix

Compute the capacity in samples once and use it on both paths:

static inline uint32_t Streaming_GetSampleCapacity(streaming_channel_t *stream)
{
    return ((uintptr_t)stream->end_ring_buffer - (uintptr_t)stream->ring_buffer) / stream->data_size;
}

Streaming_AddAvailableSampleNB now uses it too — that is a pure de-duplication of an expression it already computed inline, with no behaviour change.

Blast radius — please read before merging

Today this overruns silently. A correct check converts silent memory corruption into a firing LUOS_ASSERT. Any existing caller that has been quietly overrunning its channel will now assert instead. That is the right trade, but it is a behaviour change and it may surface as new asserts in code that appeared to work.

Because of that, Luos_ReceiveStreaming is handled differently: its size comes straight off the wire, so letting it assert would hand any remote node a reliable way to halt a peer. It now drops a chunk the channel cannot hold and returns FAILED rather than asserting. It also drops a chunk shorter than one sample — chunk_size / data_size was 0 there, which tripped the size > 0 assert in Streaming_PutSample. Both were remotely triggerable.

Note this reuses the existing FAILED return, which on this function already means "more chunks coming" rather than "error". Rejecting is the safe minimum; if you would rather it clamp and keep the samples that fit, say so and I will change it.

Sweep — found but NOT fixed here

Two further problems in the same file, both out of scope for a minimal fix. Happy to open separate PRs.

1. Luos_SendStreamingSize has the mirror-image unit bug (verified). Line 268 sets msg->header.size = data_size, where that local holds a sample count, while Luos_ReceiveStreaming reads header.size as bytes and divides by data_size. Round-trip only works when data_size == 1. Measured on a 4-byte channel: sending 10 samples put header.size = 10 on the wire, and the receiver stored 2 samples. Correction: I first flagged this as a wire-format change and therefore out of scope. That was wrong. The wire is byte-oriented by design, so writing a sample count into header.size is simply a bug, and fixing it restores the intended semantics rather than changing them. Now fixed in fix(streaming): send the streaming chunk size in bytes, not in samples #509, stacked on this PR.

2. Streaming_GetAvailableSampleNBUntilEndBuffer loop detection is unreliable on 32-bit (by inspection, not measured). Line 150 divides the pointer difference before narrowing to int32_t, unlike Streaming_GetAvailableSampleNB at line 132 which casts first. On a 32-bit target a wrapped buffer gives (2^32 - k)/data_size, which stays positive, so the < 0 branch never runs. The native tests are 64-bit, where truncation happens to yield -1, so this is invisible there. I did not verify this on hardware.

Also noted, not changed: Streaming_AddAvailableSampleNB rejects a request that would exactly fill the channel (> 0), while Streaming_PutSample allows it (<= capacity, and an existing test asserts that filling exactly must not fire). The two functions disagree by one; I preserved both behaviours rather than pick one.

Tests

Added unittest_Streaming_PutSample_capacity and unittest_Luos_ReceiveStreaming_oversized — the first streaming tests to use a data_size > 1, with an explicit guard region to catch overruns.

121 test cases: 121 succeeded    (origin/main, baseline)
123 test cases: 123 succeeded    (this branch)

Both new tests fail on unpatched main. No existing test changed. clang-format clean.

Based on origin/main. Does not touch #506 or #507.