lynxis lazus has uploaded this change for review.

View Change

hlr: use talloc for memory allocation in osmo_gsup_create_insert_subscriber_data_msg

Don't use static buffers for APN and MSISDN.
When encoding multiple APNs the static buffer might be to small.
In prepration to support multiple APNs in subscriber data

Change-Id: I00b5c2dfadcf6e0740e93b4c3292d2654d22e80c
---
M include/osmocom/hlr/gsup_server.h
M src/gsup_server.c
M src/hlr.c
M src/lu_fsm.c
4 files changed, 38 insertions(+), 34 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/11/32511/1
diff --git a/include/osmocom/hlr/gsup_server.h b/include/osmocom/hlr/gsup_server.h
index 9c562e2..86ae4ec 100644
--- a/include/osmocom/hlr/gsup_server.h
+++ b/include/osmocom/hlr/gsup_server.h
@@ -69,8 +69,6 @@
int osmo_gsup_configure_wildcard_apn(struct osmo_gsup_message *gsup,
uint8_t *apn_buf, size_t apn_buf_size);
int osmo_gsup_create_insert_subscriber_data_msg(struct osmo_gsup_message *gsup, const char *imsi, const char *msisdn,
- uint8_t *msisdn_enc, size_t msisdn_enc_size,
- uint8_t *apn_buf, size_t apn_buf_size,
- enum osmo_gsup_cn_domain cn_domain);
+ enum osmo_gsup_cn_domain cn_domain, void *talloc_ctx);
int osmo_gsup_forward_to_local_peer(struct osmo_gsup_server *server, const struct osmo_cni_peer_id *to_peer,
struct osmo_gsup_req *req, struct osmo_gsup_message *modified_gsup);
diff --git a/src/gsup_server.c b/src/gsup_server.c
index f0d8101..b14a791 100644
--- a/src/gsup_server.c
+++ b/src/gsup_server.c
@@ -446,19 +446,16 @@
* \param[out] gsup The gsup message to populate.
* \param[in] imsi The subscriber's IMSI.
* \param[in] msisdn The subscriber's MSISDN.
- * \param[out] msisdn_enc A buffer large enough to store the MSISDN in encoded form.
- * \param[in] msisdn_enc_size Size of the buffer (must be >= OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN).
- * \param[out] apn_buf A buffer large enough to store an APN (required if cn_domain is OSMO_GSUP_CN_DOMAIN_PS).
- * \param[in] apn_buf_size Size of APN buffer (must be >= APN_MAXLEN).
* \param[in] cn_domain The CN Domain of the subscriber connection.
+ * \param[in] talloc_ctx To allocation memory for dynamic fields (msisdn, apn) in the gsup field
* \returns 0 on success, and negative on error.
*/
int osmo_gsup_create_insert_subscriber_data_msg(struct osmo_gsup_message *gsup, const char *imsi, const char *msisdn,
- uint8_t *msisdn_enc, size_t msisdn_enc_size,
- uint8_t *apn_buf, size_t apn_buf_size,
- enum osmo_gsup_cn_domain cn_domain)
+ enum osmo_gsup_cn_domain cn_domain,
+ void *talloc_ctx)
{
int len;
+ char *msisdn_buf = talloc_size(talloc_ctx, OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN);

OSMO_ASSERT(gsup);
*gsup = (struct osmo_gsup_message){
@@ -467,27 +464,22 @@

osmo_strlcpy(gsup->imsi, imsi, sizeof(gsup->imsi));

- if (msisdn_enc_size < OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN)
- return -ENOSPC;
-
- OSMO_ASSERT(msisdn_enc);
- len = gsm48_encode_bcd_number(msisdn_enc, msisdn_enc_size, 0, msisdn);
+ len = gsm48_encode_bcd_number(msisdn_buf, OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN, 0, msisdn);
if (len < 1) {
LOGP(DLGSUP, LOGL_ERROR, "%s: Error: cannot encode MSISDN '%s'\n", imsi, msisdn);
return -ENOSPC;
}
- gsup->msisdn_enc = msisdn_enc;
+ gsup->msisdn_enc = msisdn_buf;
gsup->msisdn_enc_len = len;

#pragma message "FIXME: deal with encoding the following data: gsup.hlr_enc"

gsup->cn_domain = cn_domain;
if (gsup->cn_domain == OSMO_GSUP_CN_DOMAIN_PS) {
- OSMO_ASSERT(apn_buf_size >= APN_MAXLEN);
- OSMO_ASSERT(apn_buf);
+ char *apn_buf = talloc_size(talloc_ctx, APN_MAXLEN);
/* FIXME: PDP infos - use more fine-grained access control
instead of wildcard APN */
- osmo_gsup_configure_wildcard_apn(gsup, apn_buf, apn_buf_size);
+ osmo_gsup_configure_wildcard_apn(gsup, apn_buf, APN_MAXLEN);
}

return 0;
diff --git a/src/hlr.c b/src/hlr.c
index 8183d9b..9f9cb58 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -85,8 +85,7 @@

llist_for_each_entry(co, &g_hlr->gs->clients, list) {
struct osmo_gsup_message gsup = { };
- uint8_t msisdn_enc[OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN];
- uint8_t apn[APN_MAXLEN];
+ void *sub_upd_ctx = talloc_named_const(NULL, 1, "hlr_sub_upd_notify");
struct msgb *msg_out;
uint8_t *peer;
int peer_len;
@@ -102,7 +101,7 @@
peer_compare = subscr->vlr_number;
} else {
/* We have not yet received a location update from this GSUP client.*/
- continue;
+ goto cont_err;
}

peer_len = osmo_gsup_conn_ccm_get(co, &peer, IPAC_IDTAG_SERNR);
@@ -111,7 +110,7 @@
"IMSI='%s': cannot get peer name for connection %s:%u\n", subscr->imsi,
co && co->conn && co->conn->server? co->conn->server->addr : "unset",
co && co->conn && co->conn->server? co->conn->server->port : 0);
- continue;
+ goto cont_err;
}

peer_strlen = strnlen((const char*)peer, peer_len);
@@ -123,7 +122,7 @@
"IMSI %s: subscriber change: skipping %s peer %s\n",
subscr->imsi, cn_domain == OSMO_GSUP_CN_DOMAIN_PS ? "PS" : "CS",
osmo_quote_str((char*)peer, -1));
- continue;
+ goto cont_err;
}

LOGP(DLGSUP, LOGL_DEBUG,
@@ -131,14 +130,13 @@
subscr->imsi, cn_domain == OSMO_GSUP_CN_DOMAIN_PS ? "PS" : "CS",
osmo_quote_str(peer_compare, -1));

- if (osmo_gsup_create_insert_subscriber_data_msg(&gsup, subscr->imsi, subscr->msisdn, msisdn_enc,
- sizeof(msisdn_enc), apn, sizeof(apn), cn_domain) != 0) {
+ if (osmo_gsup_create_insert_subscriber_data_msg(&gsup, subscr->imsi, subscr->msisdn, cn_domain, sub_upd_ctx) != 0) {
LOGP(DLGSUP, LOGL_ERROR,
"IMSI='%s': Cannot notify GSUP client; could not create gsup message "
"for %s:%u\n", subscr->imsi,
co && co->conn && co->conn->server? co->conn->server->addr : "unset",
co && co->conn && co->conn->server? co->conn->server->port : 0);
- continue;
+ goto cont_err;
}

/* Send ISD to MSC/SGSN */
@@ -149,7 +147,7 @@
"for %s:%u\n", subscr->imsi,
co && co->conn && co->conn->server? co->conn->server->addr : "unset",
co && co->conn && co->conn->server? co->conn->server->port : 0);
- continue;
+ goto cont_err;
}
osmo_gsup_encode(msg_out, &gsup);

@@ -159,8 +157,11 @@
"for %s:%u\n", subscr->imsi,
co && co->conn && co->conn->server? co->conn->server->addr : "unset",
co && co->conn && co->conn->server? co->conn->server->port : 0);
- continue;
+ goto cont_err;
}
+ cont_err:
+ talloc_free(sub_upd_ctx);
+ continue;
}
}

diff --git a/src/lu_fsm.c b/src/lu_fsm.c
index deb180b..ef653a0 100644
--- a/src/lu_fsm.c
+++ b/src/lu_fsm.c
@@ -241,19 +241,19 @@
struct lu *lu = fi->priv;
struct hlr_subscriber *subscr = &lu->subscr;
struct osmo_gsup_message gsup;
- uint8_t msisdn_enc[OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN];
- uint8_t apn[APN_MAXLEN];
+ void *lu_isr_ctx = talloc_named_const(NULL, 1, "lu_isr");

if (osmo_gsup_create_insert_subscriber_data_msg(&gsup, subscr->imsi,
- subscr->msisdn, msisdn_enc, sizeof(msisdn_enc),
- apn, sizeof(apn),
- lu->is_ps? OSMO_GSUP_CN_DOMAIN_PS : OSMO_GSUP_CN_DOMAIN_CS)) {
+ subscr->msisdn,
+ lu->is_ps? OSMO_GSUP_CN_DOMAIN_PS : OSMO_GSUP_CN_DOMAIN_CS,
+ lu_isr_ctx)) {
lu_failure(lu, GMM_CAUSE_NET_FAIL, "cannot encode Insert Subscriber Data message");
return;
}

if (osmo_gsup_req_respond(lu->update_location_req, &gsup, false, false))
lu_failure(lu, GMM_CAUSE_NET_FAIL, "cannot send %s", osmo_gsup_message_type_name(gsup.message_type));
+ talloc_free(lu_isr_ctx);
}

void lu_fsm_wait_insert_data_result(struct osmo_fsm_inst *fi, uint32_t event, void *data)

To view, visit change 32511. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Change-Id: I00b5c2dfadcf6e0740e93b4c3292d2654d22e80c
Gerrit-Change-Number: 32511
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis@fe80.eu>
Gerrit-MessageType: newchange