From e3d03f4f14eb498b470a974bbd84b31a2df41545 Mon Sep 17 00:00:00 2001
From: Jonathan Hui <jonhui@google.com>
Date: Wed, 11 Mar 2026 14:04:02 -0500
Subject: [PATCH] [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.
---
 third_party/tcplp/lib/cbuf.c          |  4 +---
 third_party/tcplp/lib/test/Makefile   |  8 ++++----
 third_party/tcplp/lib/test/test_all.c | 27 +++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/third_party/tcplp/lib/cbuf.c b/third_party/tcplp/lib/cbuf.c
index 5662ef209..09000107f 100644
--- a/third_party/tcplp/lib/cbuf.c
+++ b/third_party/tcplp/lib/cbuf.c
@@ -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);
diff --git a/third_party/tcplp/lib/test/Makefile b/third_party/tcplp/lib/test/Makefile
index 5613cbd18..c97a359a9 100644
--- a/third_party/tcplp/lib/test/Makefile
+++ b/third_party/tcplp/lib/test/Makefile
@@ -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
diff --git a/third_party/tcplp/lib/test/test_all.c b/third_party/tcplp/lib/test/test_all.c
index 9884259f4..bbe1a40ac 100644
--- a/third_party/tcplp/lib/test/test_all.c
+++ b/third_party/tcplp/lib/test/test_all.c
@@ -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) {