pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28885 ) Change subject: cbsp: Fix heap-use-after-free closing cli conn in connecting state ...................................................................... cbsp: Fix heap-use-after-free closing cli conn in connecting state if conn is not in STREAM_CLI_STATE_CONNECTED state, it won't call disconnect_cb during osmo_stream_cli_destroy(), hence the osmo-cbc pointers holding are not nullified correctly. """ 20220801174147247 DCBSP NOTICE ttcn3-bsc-server: Disconnected. (cbsp_link.c:101) 20220801174147247 DCBSP NOTICE ttcn3-bsc-server: Reconnecting... (cbsp_link.c:102) 20220801174147247 DLINP INFO [WAIT_RECONNECT] osmo_stream_cli_reconnect(): retrying in 5 seconds... (stream.c:287) 20220801174152235 DCBSP DEBUG CBSP-Link[0x612000002c20]{RESET_PENDING}: Timeout of T3 (fsm.c:317) 20220801174152236 DCBSP DEBUG CBSP-Link[0x612000002c20]{RESET_PENDING}: timer_cb requested termination (fsm.c:327) 20220801174152236 DCBSP DEBUG CBSP-Link[0x612000002c20]{RESET_PENDING}: Terminating (cause = OSMO_FSM_TERM_TIMEOUT) (fsm.c:332) ================================================================= ==17==ERROR: AddressSanitizer: heap-use-after-free on address 0x6180000024f0 at pc 0x7fbd28a05d01 bp 0x7ffe247352b0 sp 0x7ffe247352a8 READ of size 4 at 0x6180000024f0 thread T0 #0 0x7fbd28a05d00 in osmo_stream_cli_close /tmp/libosmo-netif/src/stream.c:307 #1 0x7fbd28a0a5b3 in osmo_stream_cli_destroy /tmp/libosmo-netif/src/stream.c:714 #2 0x55c3534a0322 in cbc_cbsp_link_close /tmp/osmo-cbc/src/cbsp_link.c:356 #3 0x55c3534a16e1 in cbsp_link_fsm_cleanup /tmp/osmo-cbc/src/cbsp_link_fsm.c:199 #4 0x7fbd28bf5085 in _osmo_fsm_inst_term /tmp/libosmocore/src/fsm.c:947 #5 0x7fbd28be6881 in fsm_tmr_cb /tmp/libosmocore/src/fsm.c:332 #6 0x7fbd28bc70ab in osmo_timers_update /tmp/libosmocore/src/timer.c:269 #7 0x7fbd28bcba5b in _osmo_select_main /tmp/libosmocore/src/select.c:394 #8 0x7fbd28bcbb31 in osmo_select_main /tmp/libosmocore/src/select.c:438 #9 0x55c35348bce8 in main /tmp/osmo-cbc/src/cbc_main.c:314 #10 0x7fbd27a4cd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26d09) #11 0x55c353488ce9 in _start (/usr/local/bin/osmo-cbc+0x68ce9) """ Change-Id: Ic13578e958345207892465644b5e1f28537c032d --- M src/cbsp_link.c M src/sbcap_link.c 2 files changed, 19 insertions(+), 4 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/cbsp_link.c b/src/cbsp_link.c index 5587f58..564e134 100644 --- a/src/cbsp_link.c +++ b/src/cbsp_link.c @@ -352,10 +352,17 @@ if (!link->conn) return; - if (link->is_client) + if (link->is_client) { osmo_stream_cli_destroy(link->cli_conn); - else + if (link->peer) + link->peer->link.cbsp = NULL; + link->cli_conn = NULL; + if (link->fi) + osmo_fsm_inst_dispatch(link->fi, CBSP_LINK_E_CMD_CLOSE, NULL); + } else { osmo_stream_srv_destroy(link->srv_conn); + /* Same as waht's done for cli is done for srv in closed_cb() */ + } } /* diff --git a/src/sbcap_link.c b/src/sbcap_link.c index 07ada0c..9af842f 100644 --- a/src/sbcap_link.c +++ b/src/sbcap_link.c @@ -420,10 +420,18 @@ if (!link->conn) return; - if (link->is_client) + if (link->is_client) { osmo_stream_cli_destroy(link->cli_conn); - else + osmo_stream_cli_destroy(link->cli_conn); + if (link->peer) + link->peer->link.sbcap = NULL; + link->cli_conn = NULL; + if (link->fi) + osmo_fsm_inst_dispatch(link->fi, SBcAP_LINK_E_CMD_CLOSE, NULL); + } else { osmo_stream_srv_destroy(link->srv_conn); + /* Same as waht's done for cli is done for srv in closed_cb() */ + } } /* -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28885 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: Ic13578e958345207892465644b5e1f28537c032d Gerrit-Change-Number: 28885 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged