Neels Hofmeyr has submitted this change and it was merged.
Change subject: fix luop crash: use buffer for APN that remains valid
......................................................................
fix luop crash: use buffer for APN that remains valid
In osmo_gsup_configure_wildcard_apn(), do not compose APN into a local buffer
that becomes invalid as soon as the function exits. Instead, use a caller
provided buf.
Fixes OS#3231 crash:
==20030==ERROR: AddressSanitizer: stack-buffer-underflow on address
0x7fffffffd9c0 at pc 0x7ffff6e9b6c2 bp 0x7fffffffd900 sp 0x7fffffffd0b0
READ of size 2 at 0x7fffffffd9c0 thread T0
#0 0x7ffff6e9b6c1 (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x766c1)
#1 0x7ffff6314419 in tlv_put
../../../../src/libosmocore/include/osmocom/gsm/tlv.h:107
#2 0x7ffff6314419 in msgb_tlv_put
../../../../src/libosmocore/include/osmocom/gsm/tlv.h:299
#3 0x7ffff6314419 in encode_pdp_info
../../../../src/libosmocore/src/gsm/gsup.c:419
#4 0x7ffff6314419 in osmo_gsup_encode
../../../../src/libosmocore/src/gsm/gsup.c:535
#5 0x555555580016 in _luop_tx_gsup ../../../src/osmo-hlr/src/luop.c:54
#6 0x5555555809d8 in lu_op_tx_insert_subscr_data
../../../src/osmo-hlr/src/luop.c:264
#7 0x55555558b356 in rx_upd_loc_req ../../../src/osmo-hlr/src/hlr.c:306
#8 0x55555558b356 in read_cb ../../../src/osmo-hlr/src/hlr.c:365
#9 0x555555586671 in osmo_gsup_server_read_cb
../../../src/osmo-hlr/src/gsup_server.c:105
#10 0x7ffff5b35911 in ipa_server_conn_read
../../../src/libosmo-abis/src/input/ipa.c:356
#11 0x7ffff5b35911 in ipa_server_conn_cb
../../../src/libosmo-abis/src/input/ipa.c:387
#12 0x7ffff5e5541f in osmo_fd_disp_fds
../../../src/libosmocore/src/select.c:216
#13 0x7ffff5e5541f in osmo_select_main
../../../src/libosmocore/src/select.c:256
#14 0x5555555791b6 in main ../../../src/osmo-hlr/src/hlr.c:600
#15 0x7ffff4707a86 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21a86)
#16 0x555555579679 in _start (/usr/local/bin/osmo-hlr+0x25679)
Address 0x7fffffffd9c0 is located in stack of thread T0 at offset 16 in frame
#0 0x7ffff63131ff in osmo_gsup_encode
../../../../src/libosmocore/src/gsm/gsup.c:481
This frame has 1 object(s):
[32, 64) 'bcd_buf' <== Memory access at offset 16 underflows this variable
Related: OS#3231
Change-Id: I83d9ef2868bbb01e3f1ddb7920fe735aca172b15
---
M src/gsup_server.c
M src/gsup_server.h
M src/hlr.c
M src/luop.c
4 files changed, 15 insertions(+), 7 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/gsup_server.c b/src/gsup_server.c
index 24ba738..03a6f56 100644
--- a/src/gsup_server.c
+++ b/src/gsup_server.c
@@ -335,16 +335,19 @@
talloc_free(gsups);
}
-void osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup)
+/* Set GSUP message's pdp_infos[0] to a wildcard APN.
+ * Use the provided apn_buf to store the produced APN data. This must remain
valid until
+ * osmo_gsup_encode() is done. */
+void osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup,
+ uint8_t *apn_buf, size_t apn_buf_size)
{
int l;
- uint8_t apn[APN_MAXLEN];
- l = osmo_apn_from_str(apn, sizeof(apn), "*");
+ l = osmo_apn_from_str(apn_buf, apn_buf_size, "*");
if (l <= 0)
return;
- gsup->pdp_infos[0].apn_enc = apn;
+ gsup->pdp_infos[0].apn_enc = apn_buf;
gsup->pdp_infos[0].apn_enc_len = l;
gsup->pdp_infos[0].have_info = 1;
gsup->num_pdp_infos = 1;
diff --git a/src/gsup_server.h b/src/gsup_server.h
index 3d36bff..b52b783 100644
--- a/src/gsup_server.h
+++ b/src/gsup_server.h
@@ -53,4 +53,5 @@
void osmo_gsup_server_destroy(struct osmo_gsup_server *gsups);
-void osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup);
+void osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup,
+ uint8_t *apn_buf, size_t apn_buf_size);
diff --git a/src/hlr.c b/src/hlr.c
index 195e5d2..cab34f0 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -32,6 +32,7 @@
#include <osmocom/vty/telnet_interface.h>
#include <osmocom/vty/ports.h>
#include <osmocom/ctrl/control_vty.h>
+#include <osmocom/gsm/apn.h>
#include "db.h"
#include "hlr.h"
@@ -67,6 +68,7 @@
uint8_t *peer;
int peer_len;
uint8_t msisdn_enc[43]; /* TODO use constant; TS 24.008
10.5.4.7 */
+ uint8_t apn[APN_MAXLEN];
int len;
struct msgb *msg_out;
@@ -96,7 +98,7 @@
/* FIXME: PDP infos - use more fine-grained access
control
instead of wildcard APN */
- osmo_gsup_configure_wildcard_apn(&gsup);
+ osmo_gsup_configure_wildcard_apn(&gsup, apn,
sizeof(apn));
} else if (co->supports_cs) {
gsup.cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
} else {
diff --git a/src/luop.c b/src/luop.c
index 7150b64..54c3884 100644
--- a/src/luop.c
+++ b/src/luop.c
@@ -27,6 +27,7 @@
#include <osmocom/core/logging.h>
#include <osmocom/gsm/gsm48_ie.h>
#include <osmocom/gsm/gsup.h>
+#include <osmocom/gsm/apn.h>
#include "gsup_server.h"
#include "gsup_router.h"
@@ -233,6 +234,7 @@
{
struct osmo_gsup_message gsup;
uint8_t msisdn_enc[43]; /* TODO use constant; TS 24.008 10.5.4.7 */
+ uint8_t apn[APN_MAXLEN];
int l;
OSMO_ASSERT(luop->state == LU_S_LU_RECEIVED ||
@@ -257,7 +259,7 @@
if (luop->is_ps) {
/* FIXME: PDP infos - use more fine-grained access control
instead of wildcard APN */
- osmo_gsup_configure_wildcard_apn(&gsup);
+ osmo_gsup_configure_wildcard_apn(&gsup, apn, sizeof(apn));
}
/* Send ISD to new VLR/SGSN */
--
To view, visit https://gerrit.osmocom.org/8027
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I83d9ef2868bbb01e3f1ddb7920fe735aca172b15
Gerrit-PatchSet: 1
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
