- gerrit-log - lists.osmocom.org

Change in osmo-hnbgw[master]: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-...
by pespin 30 Sep '22

30 Sep '22

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 to look at the new patch set (#2). Change subject: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free ...................................................................... hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free Otherwise the libosmo-netif stream API may continue accessing the conn after returning if the socket has the WRITE flag active in the same main loop iteration. Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/49/29549/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e Gerrit-Change-Number: 29549 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

Change in osmo-hnbgw[master]: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-...
by pespin 30 Sep '22

30 Sep '22

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 ) Change subject: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549/comment/41070233_2ad1aa52 PS1, Line 11: loop iteration. > sounds like adapting the api implementation to fix a problem that should be fixed in the calling cod […] This is a historical usual way of handling this in lots of osmocom code, so I'm simply sticking to existing convention in place. I'm doing nothing new here. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e Gerrit-Change-Number: 29549 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:36:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmo-hnbgw[master]: Close conn when receiving SCTP_ASSOC_CHANGE notification
by neels 30 Sep '22

30 Sep '22

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547 ) Change subject: Close conn when receiving SCTP_ASSOC_CHANGE notification ...................................................................... Patch Set 1: (2 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547/comment/3e6e5a02_34b7c31a PS1, Line 9: It was seen on a real pcap trace (sctp & gsmtap_log) that the kernel i read this before and commented it, can you "link" the patches by a Related: <change-id> and explain how they relate Patchset: PS1: (looks like this function also fails to put() to msgb if sctp_revmsg() rc > 0 in case of NOTIFICATION, like seen in that other patch) -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: If35efd404405f926a4a6cc45862eeadd1b04e08c Gerrit-Change-Number: 29547 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:36:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in libosmo-netif[master]: stream: Document osmo_stream_srv_recv() SCTP specialties
by pespin 30 Sep '22

30 Sep '22

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29550 ) Change subject: stream: Document osmo_stream_srv_recv() SCTP specialties ...................................................................... stream: Document osmo_stream_srv_recv() SCTP specialties Change-Id: I47b066f26e63afd4bdb135f822667d1cd9479920 --- M src/stream.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/50/29550/1 diff --git a/src/stream.c b/src/stream.c index 38d24fe..68de30c 100644 --- a/src/stream.c +++ b/src/stream.c @@ -1524,6 +1524,16 @@ * \param[in] conn Stream Server from which to receive * \param msg pre-allocate message buffer to which received data is appended * \returns number of bytes read, negative on error. + * + * If conn is an SCTP connection, additional specific considerations shall be taken: + * - msg->cb is always filled with SCTP ppid, and SCTP stream values, see msgb_sctp_*() APIs. + * - If an SCTP notification was received when reading from the SCTP socket, + * msgb_sctp_msg_flags(msg) will contain bit flag + * OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION set, and he msgb will + * contain a "union sctp_notification" instead of user data. In this case the + * return code will be either 0 (if conn is considered dead after the + * notification) or -EAGAIN (if conn is considered still alive after the + * notification) resembling the standard recv() API. */ int osmo_stream_srv_recv(struct osmo_stream_srv *conn, struct msgb *msg) { -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29550 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I47b066f26e63afd4bdb135f822667d1cd9479920 Gerrit-Change-Number: 29550 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

Change in osmo-hnbgw[master]: hnb_read_cb: Store ofd in variable
by neels 30 Sep '22

30 Sep '22

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548 ) Change subject: hnb_read_cb: Store ofd in variable ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548/comment/e3d82cf2_257b5115 PS1, Line 7: hnb_read_cb: Store ofd in variable "use local var to reduce get_ofd() calls" -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ic7058b5a05b0d34b80617006d4e929a523212221 Gerrit-Change-Number: 29548 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:30:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in osmo-hnbgw[master]: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-...
by neels 30 Sep '22

30 Sep '22

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 ) Change subject: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549/comment/526a25ae_b3c2f706 PS1, Line 11: loop iteration. sounds like adapting the api implementation to fix a problem that should be fixed in the calling code. Why is it not enough to return more detailed negative rc, especially in the case } else if (rc < 0) { ... Now the caller always gets -EBADF, hiding the actual error cause that was returned before. "Bad file descriptor" seems inaccurate -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e Gerrit-Change-Number: 29549 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:27:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in libosmo-netif[master]: stream: Set proper msgb length when returning sctp_notification
by pespin 30 Sep '22

30 Sep '22

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29540 ) Change subject: stream: Set proper msgb length when returning sctp_notification ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > IMHO we should directly set the msgb length in all cases where sctp_recvmsg() returned a length. […] It's not returning user data, it's a really specific SCTP use case which we try to emulate through a generic API used by several protocols. I'm fine with submitting a new patch documenting it. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/29540/comment/d45cb613_b3fc71e2 PS1, Line 1563: msgb_put(msg, ret); > i'd move this step to directly after sctp_recvmsg() This is the generic and regular place where user data content is updated in the msgb, so it's fine here. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29540 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e Gerrit-Change-Number: 29540 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:22:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in libosmo-netif[master]: stream: Set proper msgb length when returning sctp_notification
by neels 30 Sep '22

30 Sep '22

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29540 ) Change subject: stream: Set proper msgb length when returning sctp_notification ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: IMHO we should directly set the msgb length in all cases where sctp_recvmsg() returned a length. Seems this is done in osmo_stream_srv_recv(), just not in cases where error is returned. Doing the put() that late leaves the msgb in incomplete state, hoping that the returned value ends up being put() in the msgb later, which i believe is the root reason for the problem that this patch fixes Also weird now is that an error is returned, but the msgb has actually received data in it. This is highly unusual behavior of a function and must be api-documented IMO. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/29540/comment/1947a7b5_18343cff PS1, Line 1563: msgb_put(msg, ret); i'd move this step to directly after sctp_recvmsg() -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29540 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e Gerrit-Change-Number: 29540 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:18:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in libosmo-netif[master]: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
by pespin 30 Sep '22

30 Sep '22

Attention is currently required from: neels. Hello Jenkins Builder, neels, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545 to look at the new patch set (#2). Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST ...................................................................... stream: Return 0 when receiving sctp notification SCTP_COMM_LOST It was seen on a real pcap trace (sctp & gsmtap_log) that the Linux kernel stack may decide to kill the connection (sending an ABORT) if it fails to transmit some data after a while: ABORT Cause code: "Protocol violation (0x000d)", Cause Information: "Association exceeded its max_retrans count". When this occurs, the kernel sends the MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when reading from the socket with sctp_recvmsg(). This basically signals that the socket conn is death, and subsequent writes to it will result in send() failures (and receive SCTP_SEND_FAILED notification upon follow up reads). It's important to notice that after those events, there's no other sort of different event like SHUTDOWN coming in, so that's the time at which we must tell the user to close the socket. Hence, let's signal the caller that the socket is dead by returning 0, to comply with usual recv() API. Related: SYS#6113 Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 --- M src/stream.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/45/29545/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 Gerrit-Change-Number: 29545 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

Change in libosmo-netif[master]: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
by pespin 30 Sep '22

30 Sep '22

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29545 ) Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmo-netif/+/29545/comment/7feb023b_bef5e9ae PS1, Line 9: kernel > probably the *Linux* kernel stack? I'm not aware that we officialy support any other kernel/OS so far. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3 Gerrit-Change-Number: 29545 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Sep 2022 14:12:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 0

2026

2025

2024

2023

2022

gerrit-log