daniel has uploaded this change for review. ( 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: Received ones get whatever was set as context in osmo_iofd_setup(). This might 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 mean time. To avoid that set the parent of any msgb in the tx queue to the iofd itself. Change-Id: I3a279b55a3adff96948120683c844e1508d0ba94 --- M src/core/osmo_io.c M src/core/osmo_io_internal.h 2 files changed, 31 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/34144/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index db1b5ad..b21c894 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -91,19 +91,19 @@ * \param[in] action the action this msg(hdr) is for (read, write, ..) * \param[in] msg the msg buffer to use. Will allocate a new one if NULL * \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 *iofd_msghdr_alloc(struct osmo_io_fd *iofd, const void *ctx, enum iofd_msg_action action, struct msgb *msg) { struct iofd_msghdr *hdr = talloc_zero(iofd, struct iofd_msghdr); if (!hdr) return NULL; if (!msg) { - msg = iofd_msgb_alloc(iofd); + msg = iofd_msgb_alloc(iofd, ctx); if (!msg) { talloc_free(hdr); return NULL; } } else { - talloc_steal(iofd->msgb_alloc.ctx, msg); + talloc_steal(ctx, msg); } hdr->action = action; @@ -124,12 +124,12 @@ } /*! convenience wrapper to call msgb_alloc with parameters from osmo_io_fd */ -struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd) +struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd, const void *ctx) { 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(ctx, iofd->msgb_alloc.size + headroom, headroom, iofd->name ? : "iofd_msgb"); } @@ -152,7 +152,7 @@ msg = iofd_msgb_pending(iofd); if (!msg) - msg = iofd_msgb_alloc(iofd); + msg = iofd_msgb_alloc(iofd, iofd->msgb_alloc.ctx); return msg; } @@ -262,7 +262,7 @@ /* msgb contains more than one segment */ /* Copy the trailing data over */ - msg_pending = iofd_msgb_alloc(iofd); + msg_pending = iofd_msgb_alloc(iofd, iofd->msgb_alloc.ctx); memcpy(msgb_data(msg_pending), data + expected_len, extra_len); msgb_put(msg_pending, extra_len); *pending_out = msg_pending; @@ -317,10 +317,11 @@ int osmo_iofd_write_msgb(struct osmo_io_fd *iofd, struct msgb *msg) { int rc; - struct iofd_msghdr *msghdr = iofd_msghdr_alloc(iofd, IOFD_ACT_WRITE, msg); + struct iofd_msghdr *msghdr = iofd_msghdr_alloc(iofd, iofd, IOFD_ACT_WRITE, msg); if (!msghdr) return -ENOMEM; + msghdr->flags = MSG_NOSIGNAL; msghdr->iov[0].iov_base = msgb_data(msghdr->msg); msghdr->iov[0].iov_len = msgb_length(msghdr->msg); @@ -355,7 +356,7 @@ OSMO_ASSERT(iofd->mode == OSMO_IO_FD_MODE_RECVFROM_SENDTO); - struct iofd_msghdr *msghdr = iofd_msghdr_alloc(iofd, IOFD_ACT_SENDTO, msg); + struct iofd_msghdr *msghdr = iofd_msghdr_alloc(iofd, iofd, IOFD_ACT_SENDTO, msg); if (!msghdr) return -ENOMEM; diff --git a/src/core/osmo_io_internal.h b/src/core/osmo_io_internal.h index d45b161..9b76cbd 100644 --- a/src/core/osmo_io_internal.h +++ b/src/core/osmo_io_internal.h @@ -126,10 +126,10 @@ IOFD_SEG_ACT_DEFER, }; -struct iofd_msghdr *iofd_msghdr_alloc(struct osmo_io_fd *iofd, enum iofd_msg_action action, struct msgb *msg); +struct iofd_msghdr *iofd_msghdr_alloc(struct osmo_io_fd *iofd, const void *ctx, enum iofd_msg_action action, struct msgb *msg); void iofd_msghdr_free(struct iofd_msghdr *msghdr); -struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd); +struct msgb *iofd_msgb_alloc(struct osmo_io_fd *iofd, const void *ctx); struct msgb *iofd_msgb_pending(struct osmo_io_fd *iofd); struct msgb *iofd_msgb_pending_or_alloc(struct osmo_io_fd *iofd); -- 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: 1 Gerrit-Owner: daniel &lt;dwillmann(a)sysmocom.de&gt; Gerrit-MessageType: newchange