Change in openbsc[master]: bsc_nat.c: Return correct err code to avoid heap-user-after-free

[Pau Espin Pedrol]

Pau Espin Pedrol has uploaded this change for review. ( 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(-)



  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/81/9381/1

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: newchange
Gerrit-Change-Id: I120f646601bd4275b9088d0d73000ce04564bc6b
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180530/e0384a4c/attachment.htm>