daniel has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34144 ) Change subject: osmo_io: Avoid potential double free when sending msgb ...................................................................... osmo_io: Avoid potential double free when sending msgb Ensure that a msgb has the proper talloc parent: All msgbs inside an iofd get the iofd as parent. Received msgbs are reparented to iofd->msgb_alloc.ctx (which was set in osmo_iofd_setup()) before being passed to the receive callback. Before this change the code could fail for msgbs that are submitted via uring where the (failed) write returns after the iofd has already been osmo_iofd_free()d. free()ing the iofd is deferred until the write completes, but the (iofd) parent context could have been free()d in the meantime. Change-Id: I3a279b55a3adff96948120683c844e1508d0ba94 --- M src/core/osmo_io.c 1 file changed, 36 insertions(+), 8 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve daniel: Looks good to me, approved diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 5fed19e..bccf7af 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -93,17 +93,23 @@ * \returns the newly allocated msghdr or NULL in case of error */ struct iofd_msghdr *iofd_msghdr_alloc(struct osmo_io_fd *iofd, enum iofd_msg_action action, struct msgb *msg) { - struct iofd_msghdr *hdr = talloc_zero(iofd, struct iofd_msghdr); - if (!hdr) - return NULL; + bool free_msg = false; + struct iofd_msghdr *hdr; + if (!msg) { msg = iofd_msgb_alloc(iofd); - if (!msg) { - talloc_free(hdr); + if (!msg) return NULL; - } + free_msg = true; } else { - talloc_steal(iofd->msgb_alloc.ctx, msg); + talloc_steal(iofd, msg); + } + + hdr = talloc_zero(msg, struct iofd_msghdr); + if (!hdr) { + if (free_msg) + talloc_free(msg); + return NULL; } hdr->action = action; @@ -129,7 +135,7 @@ uint16_t headroom = iofd->msgb_alloc.headroom; OSMO_ASSERT(iofd->msgb_alloc.size < 0xffff - headroom); - return msgb_alloc_headroom_c(iofd->msgb_alloc.ctx, + return msgb_alloc_headroom_c(iofd, iofd->msgb_alloc.size + headroom, headroom, iofd->name ? : "iofd_msgb"); } @@ -304,6 +310,7 @@ void iofd_handle_recv(struct osmo_io_fd *iofd, struct msgb *msg, int rc, struct iofd_msghdr *hdr) { + talloc_steal(iofd->msgb_alloc.ctx, msg); switch (iofd->mode) { case OSMO_IO_FD_MODE_READ_WRITE: iofd_handle_segmented_read(iofd, msg, rc); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34144 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3a279b55a3adff96948120683c844e1508d0ba94 Gerrit-Change-Number: 34144 Gerrit-PatchSet: 4 Gerrit-Owner: daniel &lt;dwillmann(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel &lt;dwillmann(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged