[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, laforge. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 4 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 20 Feb 2024 07:40:36 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, jolly, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 4: (3 comments) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/6e1903f0_dfe8c37c PS4, Line 199: * If there is pending data to sent, iofd_uring_submit_tx will attach it again. to send https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/cd22fe2e_01046fff PS4, Line 201: * iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. */ "(which in turn frees this msghdr, if still attached) and frees the msghdr". you are repeating yourself here? https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/4f084d16_4f109c36 PS4, Line 210: } where does the second free happen? in iofd_uring_submit_tx? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 4 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 20 Feb 2024 10:27:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, jolly, laforge. Hello Jenkins Builder, daniel, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by laforge, Verified+1 by Jenkins Builder Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() msghdr must be detached, because subsequent callback at iofd_handle_send_completion() may destroy the iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. Related: OS#5751 Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 --- M src/core/osmo_io_uring.c 1 file changed, 22 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/82/35982/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 5 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, laforge, pespin. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 5: (3 comments) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/26249b26_472c84e4 PS4, Line 199: * If there is pending data to sent, iofd_uring_submit_tx will attach it again. > to send Done https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/1a334c36_97fe3d5a PS4, Line 201: * iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. */ > "(which in turn frees this msghdr, if still attached) and frees the msghdr". > […] Done https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/cf975e1a_03c22e8a PS4, Line 210: } > where does the second free happen? in iofd_uring_submit_tx? I rephrased it. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 5 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 22 Feb 2024 10:00:39 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: jolly, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 5 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 22 Feb 2024 10:51:04 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: jolly, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 6: (1 comment) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/b6899b5d_40420fcb PS6, Line 200: * iofd_handle_send_completion() will free msghdr at the end. the previous callback function may destroy iofd. Which "previous callback function" are you talking about here? I'm still not fully understanding what and how gets freed by reading all this sorry. And I think it makes sense that it becomes clear before getting merged, so that whoever reads this fully understands what's going on. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Comment-Date: Thu, 22 Feb 2024 15:42:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: jolly, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 7: (1 comment) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/6148051c_3ac61dca PS6, Line 200: * iofd_handle_send_completion() will free msghdr at the end. the previous callback function may destroy iofd. > Which "previous callback function" are you talking about here? […] Still waiting for this to be clarified. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 7 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Comment-Date: Fri, 23 Feb 2024 13:29:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, jolly, laforge. Hello Jenkins Builder, daniel, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Code-Review+1 by daniel, Verified+1 by Jenkins Builder Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() msghdr must be detached, because subsequent callback at iofd_handle_send_completion() may destroy the iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. Related: OS#5751 Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 --- M src/core/osmo_io_uring.c 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/82/35982/9 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: laforge Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, laforge, pespin. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 9: (1 comment) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35982/comment/239e58ef_242b6b4b PS6, Line 200: * iofd_handle_send_completion() will free msghdr at the end. the previous callback function may destroy iofd. > Still waiting for this to be clarified. Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 29 Feb 2024 08:17:39 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, jolly, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 9: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 29 Feb 2024 11:58:16 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
Attention is currently required from: daniel, jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. Patch Set 9: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-Attention: daniel Gerrit-Comment-Date: Thu, 29 Feb 2024 12:56:59 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...
laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email ) Change subject: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() .. osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_send_completion() msghdr must be detached, because subsequent callback at iofd_handle_send_completion() may destroy the iofd (which in turn frees this msghdr, if still attached) and frees the msghdr, causing a double free. Related: OS#5751 Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 --- M src/core/osmo_io_uring.c 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 3812b50..cb636da 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -195,6 +195,15 @@ { struct osmo_io_fd *iofd = msghdr->iofd; + /* Detach msghdr from iofd. It might get freed here or it is freed during iofd_handle_send_completion(). +* If there is pending data to send, iofd_uring_submit_tx() will attach it again. +* iofd_handle_send_completion() will invoke a callback function to signal the possibility of write/send. +* This callback function might close iofd, leading to the potential freeing of iofd->u.uring.write_msghdr if +* still attached. Since iofd_handle_send_completion() frees msghdr at the end of the function, detaching +* msghdr here prevents a double-free bug. */ + if (iofd->u.uring.write_msghdr == msghdr) + iofd->u.uring.write_msghdr = NULL; + if (OSMO_UNLIKELY(IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED))) { msgb_free(msghdr->msg); iofd_msghdr_free(msghdr); @@ -202,7 +211,6 @@ iofd_handle_send_completion(iofd, rc, msghdr); } - iofd->u.uring.write_msghdr = NULL; /* submit the next to-be-transmitted message for this file descriptor */ if (iofd->u.uring.write_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_submit_tx(iofd); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35982?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia349f73de2145fa360b20dd40deb73a8ffc71f07 Gerrit-Change-Number: 35982 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: merged