Harald Welte has submitted this change and it was merged. (
https://gerrit.osmocom.org/9381 )
Change subject: bsc_nat.c: Return correct err code to avoid heap-user-after-free
......................................................................
bsc_nat.c: Return correct err code to avoid heap-user-after-free
When ipaccess_bsc_read_cb calls bsc_close_connection, the osmo_fd
struct is freed, so we need to indicate to osmo_wqueue_bfd_cb that it
should not continue using the fd pointer after we return.
Fixes following AdressSanitizer report:
<0015> openbsc/openbsc/src/osmo-bsc_nat/bsc_nat.c:1317 The connection to the
BSC Nr: -1 was lost. Cleaning it
=================================================================
==27028==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000c521c
at pc 0x7ffff606b056 bp 0x7fffffffe170 sp 0x7fffffffe168
READ of size 4 at 0x6160000c521c thread T0
#0 0x7ffff606b055 in osmo_wqueue_bfd_cb libosmocore/src/write_queue.c:65
#1 0x7ffff6055c3b in osmo_fd_disp_fds libosmocore/src/select.c:217
#2 0x7ffff6055ed5 in osmo_select_main libosmocore/src/select.c:257
#3 0x421c82 in main openbsc/openbsc/src/osmo-bsc_nat/bsc_nat.c:1713
#4 0x7ffff4803b44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
#5 0x406438 (/bin/osmo-bsc_nat+0x406438)
Fixes: OS#3300
Change-Id: I120f646601bd4275b9088d0d73000ce04564bc6b
---
M openbsc/src/osmo-bsc_nat/bsc_nat.c
1 file changed, 16 insertions(+), 15 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c
b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index 57b51a2..38a29be 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -1308,20 +1308,18 @@
int ret;
ret = ipa_msg_recv_buffered(bfd->fd, &msg, &bsc->pending_msg);
- if (ret <= 0) {
- if (ret == -EAGAIN)
- return 0;
- if (ret == 0)
- LOGP(DNAT, LOGL_ERROR,
- "The connection to the BSC Nr: %d was lost.
Cleaning it\n",
- bsc->cfg ? bsc->cfg->nr : -1);
- else
- LOGP(DNAT, LOGL_ERROR,
- "Stream error on BSC Nr: %d. Failed to parse ip
access message: %d (%s)\n",
- bsc->cfg ? bsc->cfg->nr : -1, ret, strerror(-ret));
-
- bsc_close_connection(bsc);
- return -1;
+ if (ret == -EAGAIN) {
+ return 0;
+ } else if (ret == 0) {
+ LOGP(DNAT, LOGL_ERROR,
+ "The connection to the BSC Nr: %d was lost. Cleaning it\n",
+ bsc->cfg ? bsc->cfg->nr : -1);
+ goto close_fd;
+ } else if (ret < 0) {
+ LOGP(DNAT, LOGL_ERROR,
+ "Stream error on BSC Nr: %d. Failed to parse ip access
message: %d (%s)\n",
+ bsc->cfg ? bsc->cfg->nr : -1, ret, strerror(-ret));
+ goto close_fd;
}
@@ -1356,8 +1354,11 @@
/* FIXME: Currently no PONG is sent to the BSC */
/* FIXME: Currently no ID ACK is sent to the BSC */
forward_sccp_to_msc(bsc, msg);
-
return 0;
+
+close_fd:
+ bsc_close_connection(bsc);
+ return -EBADF;
}
static int ipaccess_listen_bsc_cb(struct osmo_fd *bfd, unsigned int what)
--
To view, visit https://gerrit.osmocom.org/9381
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I120f646601bd4275b9088d0d73000ce04564bc6b
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder
