[tcplp] fix boundary check in cbuf_reass_write (#12671)

This commit fixes a logic error in the TCP receive buffer reassembly
logic. The issue occurred when an out-of-order segment was exactly
the size of the circular buffer and the write index was non-zero.

The original logic incorrectly used modulo-wrapped indices to check
if a write should be contiguous or split:
start_index + numbytes % size. When numbytes == size, end_index ==
start_index, which evaluates to true, leading to an incorrect memory
write if start_index > 0.

This commit updates the check to use the absolute write boundary:
if (start_index + numbytes <= chdr->size). This ensures that any
write spanning the buffer boundary is correctly split.

A regression test test_cbuf_reass_boundary is added to test_all.c
to verify the fix and prevent future regressions. The test Makefile
is also updated to use $(CC) for better portability.
This commit is contained in:
Jonathan Hui
2026-03-11 14:04:02 -05:00
committed by GitHub
parent edd387d04e
commit e3d03f4f14
3 changed files with 32 additions and 7 deletions
+1 -3
View File
@@ -287,7 +287,6 @@ size_t cbuf_reass_write(struct cbufhead* chdr, size_t offset, const void* data,
uint8_t* buf_data = chdr->buf;
size_t free_space = cbuf_free_space(chdr);
size_t start_index;
size_t end_index;
size_t bytes_to_end;
if (offset > free_space) {
return 0;
@@ -295,8 +294,7 @@ size_t cbuf_reass_write(struct cbufhead* chdr, size_t offset, const void* data,
numbytes = free_space - offset;
}
start_index = (cbuf_get_w_index(chdr) + offset) % chdr->size;
end_index = (start_index + numbytes) % chdr->size;
if (end_index >= start_index) {
if (numbytes <= chdr->size - start_index) {
copy_from(buf_data, start_index, data, data_offset, numbytes);
if (bitmap) {
bmp_setrange(bitmap, start_index, numbytes);
+4 -4
View File
@@ -1,16 +1,16 @@
CC=clang
CC?=gcc
CFLAGS=-I ../../../../include -O2 -Wall
all: test_all
%.o: ../%.c
clang -c $(CFLAGS) $< -o $@
$(CC) -c $(CFLAGS) $< -o $@
test_all.o: test_all.c
clang -c $(CFLAGS) test_all.c -o $@
$(CC) -c $(CFLAGS) test_all.c -o $@
test_all: test_all.o cbuf.o lbuf.o bitmap.o
clang test_all.o cbuf.o lbuf.o bitmap.o -o test_all
$(CC) test_all.o cbuf.o lbuf.o bitmap.o -o test_all
clean:
rm -f *.o test_all
+27
View File
@@ -206,10 +206,37 @@ void test_cbuf_2() {
bmp_test("cbuf_reass_merge (bitmap)", bitmap, 4, "00000000");
}
void test_cbuf_reass_boundary() {
uint8_t buffer[32];
uint8_t bitmap[4];
struct cbufhead chdr;
char data[32];
memset(data, 'A', 32);
cbuf_init(&chdr, buffer, 32);
// Set w_index to 10 by writing 10 bytes and then popping them
cbuf_write(&chdr, "0123456789", 0, 10, cbuf_copy_into_buffer);
cbuf_pop(&chdr, 10);
bmp_init(bitmap, 4);
// Write 32 bytes (full buffer size) out-of-order at offset 0
// This should trigger the wrap-around logic safely instead of overflowing
cbuf_reass_write(&chdr, 0, data, 0, 32, bitmap, NULL, cbuf_copy_from_buffer);
// If it didn't crash, it's already a good sign.
// Let's verify we can merge it and see the result.
cbuf_reass_merge(&chdr, 32, bitmap);
cbuf_test("test_cbuf_reass_boundary", &chdr, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
bmp_test("test_cbuf_reass_boundary (bitmap)", bitmap, 4, "00000000");
}
int main(int argc, char** argv) {
test_bmp();
test_cbuf();
test_cbuf_2();
test_cbuf_reass_boundary();
printf("%" PRIu32 " tests passed (out of %" PRIu32 ")\n", num_tests_passed, num_tests_passed + num_tests_failed);
if (num_tests_failed != 0) {