From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 7 May 2026 18:23:09 +0000 (-0400)
Subject: network: fix uncommitted reservation aliasing when app_io->read returns no data
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46e852e6658f3f7dc74ec512febeece53b97a322;p=thirdparty%2Ffreeradius-server.git

network: fix uncommitted reservation aliasing when app_io->read returns no data

fr_message_and_data_reserve() uses fr_ring_buffer_reserve() which does not
advance write_offset. If app_io->read() returns 0 (e.g. ldap_sync, which
does its own reads internally) and the cached cd is held in s->cd, any
subsequent allocation on the same message set returns the same ring-buffer
slot and zeroes it, corrupting cd->m.data.

Fix: only cache s->cd when s->leftover > 0 (partial stream data to preserve).
With no leftover, cancel the reservation explicitly via fr_message_and_data_reset()
which clears the message fields and marks the slot FR_MESSAGE_FREE so the next
reserve can reclaim it cleanly.

Also fix the B2 commit size in fr_network_read(): data_size returned by TCP
app_io already includes the leftover bytes already in the buffer, so the old
cd->m.data_size + data_size was double-counting.
---

diff --git a/src/lib/io/message.c b/src/lib/io/message.c
index d9b5444d2ee..702ea5edce2 100644
--- a/src/lib/io/message.c
+++ b/src/lib/io/message.c
@@ -1009,6 +1009,26 @@ fr_message_t *fr_message_and_data_reserve(fr_message_set_t *ms, size_t reserve_s
 	return message_data_reserve(ms, m, cleaned_up);
 }
 
+/** Cancel a reservation made by fr_message_and_data_reserve(), returning the slot to the set.
+ *
+ *  Both rings are uncommitted at this point (write_offset was not advanced).
+ *  Clearing the message fields is sufficient: the next reserve overwrites the
+ *  slot, and the data ring's reserved field is replaced by the new reservation size.
+ *
+ * @param[in] ms  the message set the reservation was made against.
+ * @param[in] m   the previously reserved message to cancel.
+ */
+void fr_message_and_data_reset(fr_message_set_t *ms, fr_message_t *m)
+{
+	(void) talloc_get_type_abort(ms, fr_message_set_t);
+	fr_assert(m->status == FR_MESSAGE_USED);
+	m->status = FR_MESSAGE_FREE;
+	m->rb = NULL;
+	m->data = NULL;
+	m->data_size = 0;
+	m->rb_size = 0;
+}
+
 /** Commit a previously reserved message, allocating exactly total_size bytes of packet data.
  *
  *  Commits both the message ring slot and the data ring buffer.  total_size is the
diff --git a/src/lib/io/message.h b/src/lib/io/message.h
index 687317f91bf..d2da6e7e2c4 100644
--- a/src/lib/io/message.h
+++ b/src/lib/io/message.h
@@ -54,6 +54,7 @@ typedef struct {
 fr_message_set_t *fr_message_set_create(TALLOC_CTX *ctx, int num_messages, size_t message_size, size_t ring_buffer_size, bool unlimited_size) CC_HINT(nonnull);
 
 fr_message_t *fr_message_and_data_reserve(fr_message_set_t *ms, size_t reserve_size) CC_HINT(nonnull);
+void          fr_message_and_data_reset(fr_message_set_t *ms, fr_message_t *m) CC_HINT(nonnull);
 fr_message_t *fr_message_and_data_commit(fr_message_set_t *ms, fr_message_t *m, size_t total_size) CC_HINT(nonnull(1));
 fr_message_t *fr_message_and_data_alloc(fr_message_set_t *ms, size_t size) CC_HINT(nonnull);
 fr_message_t *fr_message_and_data_commit_with_leftover(fr_message_set_t *ms, fr_message_t *m, size_t actual_packet_size,
diff --git a/src/lib/io/network.c b/src/lib/io/network.c
index b4ac252e45c..fc0734f3de0 100644
--- a/src/lib/io/network.c
+++ b/src/lib/io/network.c
@@ -949,17 +949,24 @@ next_message:
 					    cd->m.data, cd->m.rb_size, &s->leftover);
 	if (data_size == 0) {
 		/*
-		 *	Cache the message for later.  This is
-		 *	important for stream sockets, which can do
-		 *	partial reads into the current buffer.  We
-		 *	need to be able to give the same buffer back
-		 *	to the stream socket for subsequent reads.
+		 *	Cache the message only when there are leftover
+		 *	bytes from a partial stream read.  The buffer
+		 *	must be handed back on the next call so the
+		 *	stream can append to it.
 		 *
-		 *	Since we have a message set for each
-		 *	fr_io_socket_t, no "head of line"
-		 *	blocking issues can happen for stream sockets.
+		 *	Without leftover bytes the reservation is
+		 *	discarded; the next call makes a fresh one.
+		 *	Holding an uncommitted reservation is unsafe:
+		 *	any allocation on the same message set (e.g.
+		 *	from a callback inside app_io->read) returns
+		 *	the same ring-buffer slot and zeroes it out.
 		 */
-		s->cd = cd;
+		if (s->leftover) {
+			s->cd = cd;
+		} else {
+			fr_message_and_data_reset(s->ms, &cd->m);
+			s->cd = NULL;
+		}
 		return;
 	}