pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/41911?usp=email )
Change subject: osmo_io: Prevent abort allocating rx segment bigger than UINT16_MAX ......................................................................
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: */