[S] Change in libosmocore[master]: osmo_io_uring: Detach msghdr from iofd before calling iofd_handle_sen...

2024-02-19 Thread jolly 
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...

2024-02-20 Thread pespin 
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...

2024-02-22 Thread jolly 
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...

2024-02-22 Thread jolly 
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...

2024-02-22 Thread daniel 
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...

2024-02-22 Thread pespin 
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...

2024-02-23 Thread pespin 
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...

2024-02-29 Thread jolly 
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...

2024-02-29 Thread jolly 
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...

2024-02-29 Thread laforge 
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...

2024-02-29 Thread pespin 
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...

2024-02-29 Thread laforge 
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