From 86c3c9efcc2a81df03a31274a3e189f4841208c7 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Wed, 30 May 2018 11:50:26 +0200 Subject: 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 --- openbsc/src/osmo-bsc_nat/bsc_nat.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'openbsc/src') diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index 57b51a2e3..38a29bec2 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -1308,20 +1308,18 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd) 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 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd) /* 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) -- cgit v1.2.3