pespin has uploaded this change for review.

View Change

osmo_io: Prevent abort allocating rx segment bigger than UINT16_MAX

Avoid hitting iofd_msgb_alloc2() assert "OSMO_ASSERT(size + headroom <=
0xffff);".

Our msgb implementation can only handle sizes up to 16bit in length,
counting headroom + data length.

Related: SYS#7842
Change-Id: I8c11a7edbed2335ada40a1f93d081041974c3586
---
M src/core/osmo_io.c
1 file changed, 18 insertions(+), 5 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/11/41911/1

diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c
index d6cd57f..5ef95e9 100644
--- a/src/core/osmo_io.c
+++ b/src/core/osmo_io.c
@@ -31,6 +31,7 @@
 #include <string.h>
 #include <stdbool.h>
 #include <errno.h>
+#include <inttypes.h>
 
 #include <osmocom/core/osmo_io.h>
 #include <osmocom/core/linuxlist.h>
@@ -357,6 +358,12 @@
 	return IOFD_SEG_ACT_DEFER;
 }
 
+static void _call_read_cb(struct osmo_io_fd *iofd, int rc, struct msgb *msg)
+{
+	talloc_steal(iofd->msgb_alloc.ctx, msg);
+	iofd->io_ops.read_cb(iofd, rc, msg);
+}
+
 /*! Restore message boundaries on read() and pass individual messages to the read callback
  */
 void iofd_handle_segmented_read(struct osmo_io_fd *iofd, int rc, struct msgb *msg)
@@ -367,8 +374,7 @@
 	OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_READ_WRITE);
 
 	if (rc <= 0) {
-		talloc_steal(iofd->msgb_alloc.ctx, msg);
-		iofd->io_ops.read_cb(iofd, rc, msg);
+		_call_read_cb(iofd, rc, msg);
 		return;
 	}
 
@@ -376,9 +382,17 @@
 	 * If the pending message is not large enough, create a larger message. */
 	if (OSMO_UNLIKELY(iofd->pending)) {
 		if (OSMO_UNLIKELY(msgb_tailroom(iofd->pending) < msgb_length(msg))) {
+			size_t new_len = msgb_length(iofd->pending) + msgb_length(msg);
 			/* Data of msg does not fit into pending message. Allocate a new message that is larger.
 			 * This implies that msgb_length(iofd->pending) + msgb_length(msg) > iofd.msgb_alloc.size. */
-			pending = iofd_msgb_alloc2(iofd, msgb_length(iofd->pending) + msgb_length(msg));
+			if (new_len + iofd->msgb_alloc.headroom > UINT16_MAX) {
+				LOGPIO(iofd, LOGL_ERROR,
+				       "Rx segmented msgb of %zu > (%u + %" PRIu16 ") bytes is unsupported, check your segment_cb!\n",
+				       new_len, iofd->msgb_alloc.headroom, UINT16_MAX);
+				_call_read_cb(iofd, -EPROTO, msg);
+				return;
+			}
+			pending = iofd_msgb_alloc2(iofd, new_len);
 			OSMO_ASSERT(pending);
 			memcpy(msgb_put(pending, msgb_length(iofd->pending)), msgb_data(iofd->pending),
 			       msgb_length(iofd->pending));
@@ -398,8 +412,7 @@
 			/* It it expected as per API spec that we return the
 			 * return value of read here. The amount of bytes in msg is
 			 * available to the user in msg itself. */
-			talloc_steal(iofd->msgb_alloc.ctx, msg);
-			iofd->io_ops.read_cb(iofd, rc, msg);
+			_call_read_cb(iofd, rc, msg);
 			/* The user could unregister/close the iofd during read_cb() above.
 			 * Once that's done, it doesn't expect to receive any more events,
 			 * so discard it: */

To view, visit change 41911. To unsubscribe, or for help writing mail filters, visit settings.