This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
laforge gerrit-no-reply at lists.osmocom.orglaforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/19768 )
Change subject: paging: pass struct osmo_mobile_identity, not encoded IE bytes
......................................................................
paging: pass struct osmo_mobile_identity, not encoded IE bytes
In get_paging_mi(), before this, an encoded buffer of Mobile Identity bytes is
returned. Code paths following this repeatedly decode the Mobile Identity
bytes, e.g. for logging. Also, in get_paging_mi(), since the TMSI is read in
from a different encoding than a typical Mobile Identity IE, the TMSI was
manually encoded into a typical Mobile Identity IE. This is essentially a code
dup of osmo_mobile_identity_encode(). Stop this madness.
Instead, in get_paging_mi(), return a decoded struct osmo_mobile_identity. Code
paths after this use the struct osmo_mobile_identity directly without repeated
decoding.
At the point of finally needing an encoded Mobile Identity IE (in
Encoding::write_paging_request()), do a proper osmo_mobile_identity_encode().
Since this may return errors, add an rc check for the caller of
write_paging_request(), gprs_rlcmac_paging_request().
A side effect is stricter validation of the Mobile Identity passing through the
Paging code path. Before, invalid MI might have passed through unnoticed.
Change-Id: Iad845acb0096b75dc453105c9c16b2252879b4ca
---
M src/bts.cpp
M src/bts.h
M src/encoding.cpp
M src/encoding.h
M src/gprs_bssgp_pcu.cpp
M src/gprs_rlcmac.cpp
M src/gprs_rlcmac.h
M src/pcu_l1_if.cpp
M src/pdch.cpp
M src/pdch.h
10 files changed, 58 insertions(+), 40 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/bts.cpp b/src/bts.cpp
index c415dd4..76ca1b0 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -355,7 +355,7 @@
m_pollController.expireTimedout(fn, max_delay);
}
-int BTS::add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len)
+int BTS::add_paging(uint8_t chan_needed, const struct osmo_mobile_identity *mi)
{
uint8_t l, trx, ts, any_tbf = 0;
struct gprs_rlcmac_tbf *tbf;
@@ -370,10 +370,8 @@
};
if (log_check_level(DRLCMAC, LOGL_INFO)) {
- struct osmo_mobile_identity omi = {};
char str[64];
- osmo_mobile_identity_decode(&omi, mi, mi_len, true);
- osmo_mobile_identity_to_str_buf(str, sizeof(str), &omi);
+ osmo_mobile_identity_to_str_buf(str, sizeof(str), mi);
LOGP(DRLCMAC, LOGL_INFO, "Add RR paging: chan-needed=%d MI=%s\n", chan_needed, str);
}
@@ -419,7 +417,7 @@
for (ts = 0; ts < 8; ts++) {
if ((slot_mask[trx] & (1 << ts))) {
/* schedule */
- if (!m_bts.trx[trx].pdch[ts].add_paging(chan_needed, mi, mi_len))
+ if (!m_bts.trx[trx].pdch[ts].add_paging(chan_needed, mi))
return -ENOMEM;
LOGP(DRLCMAC, LOGL_INFO, "Paging on PACCH of TRX=%d TS=%d\n", trx, ts);
diff --git a/src/bts.h b/src/bts.h
index edccc28..5a1b162 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -309,7 +309,7 @@
int current_frame_number() const;
/** add paging to paging queue(s) */
- int add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len);
+ int add_paging(uint8_t chan_needed, const struct osmo_mobile_identity *mi);
gprs_rlcmac_dl_tbf *dl_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
gprs_rlcmac_ul_tbf *ul_tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts);
diff --git a/src/encoding.cpp b/src/encoding.cpp
index 9dfd7c9..881dc61 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -30,6 +30,7 @@
extern "C" {
#include <osmocom/gprs/protocol/gsm_04_60.h>
#include <osmocom/gsm/protocol/gsm_04_08.h>
+#include <osmocom/gsm/gsm48.h>
}
#include <stdbool.h>
@@ -720,8 +721,10 @@
}
/* Generate paging request. See 44.018, sections 10 and 9.1.22 */
-int Encoding::write_paging_request(bitvec * dest, const uint8_t *mi, uint8_t mi_len)
+int Encoding::write_paging_request(bitvec * dest, const struct osmo_mobile_identity *mi)
{
+ uint8_t mi_buf[GSM48_MID_MAX_SIZE];
+ int mi_len;
unsigned wp = 0;
int plen;
@@ -732,8 +735,11 @@
bitvec_write_field(dest, &wp,0x0,4); // Page Mode
bitvec_write_field(dest, &wp,0x0,4); // Channel Needed
+ mi_len = osmo_mobile_identity_encode_buf(mi_buf, sizeof(mi_buf), mi, true);
+ if (mi_len <= 0)
+ return mi_len;
bitvec_write_field(dest, &wp, mi_len, 8); // Mobile Identity length
- bitvec_set_bytes(dest, mi, mi_len); // Mobile Identity
+ bitvec_set_bytes(dest, mi_buf, mi_len); // Mobile Identity
wp += mi_len * 8;
OSMO_ASSERT(wp % 8 == 0);
diff --git a/src/encoding.h b/src/encoding.h
index 5bdd4ef..2f6b273 100644
--- a/src/encoding.h
+++ b/src/encoding.h
@@ -78,7 +78,7 @@
bitvec * dest, struct gprs_rlcmac_ul_tbf *tbf, bool is_final,
uint8_t rrbp);
- static int write_paging_request(bitvec * dest, const uint8_t *mi, uint8_t mi_len);
+ static int write_paging_request(bitvec * dest, const struct osmo_mobile_identity *mi);
static unsigned write_repeated_page_info(bitvec * dest, unsigned& wp, uint8_t len,
uint8_t *identity, uint8_t chan_needed);
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index a46852a..14d1a9d 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -171,11 +171,8 @@
}
/* Returns 0 on success, suggested BSSGP cause otherwise */
-static unsigned int get_paging_mi(const uint8_t **mi, uint8_t *mi_len,
- const struct tlv_parsed *tp)
+static unsigned int get_paging_mi(struct osmo_mobile_identity *mi, const struct tlv_parsed *tp)
{
- static uint8_t tmsi_buf[GSM48_TMSI_LEN];
-
/* Use TMSI (if present) or IMSI */
if (TLVP_PRESENT(tp, BSSGP_IE_TMSI)) {
/* Be safe against an evil SGSN - check the length */
@@ -185,14 +182,17 @@
}
/* NOTE: TMSI (unlike IMSI) IE comes without MI type header */
- memcpy(tmsi_buf + 1, TLVP_VAL(tp, BSSGP_IE_TMSI), GSM23003_TMSI_NUM_BYTES);
- tmsi_buf[0] = 0xf0 | GSM_MI_TYPE_TMSI;
-
- *mi_len = GSM48_TMSI_LEN;
- *mi = tmsi_buf;
+ *mi = (struct osmo_mobile_identity){
+ .type = GSM_MI_TYPE_TMSI,
+ };
+ mi->tmsi = osmo_load32be(TLVP_VAL(tp, BSSGP_IE_TMSI));
} else if (TLVP_PRESENT(tp, BSSGP_IE_IMSI)) {
- *mi_len = TLVP_LEN(tp, BSSGP_IE_IMSI);
- *mi = TLVP_VAL(tp, BSSGP_IE_IMSI);
+ int rc = osmo_mobile_identity_decode(mi, TLVP_VAL(tp, BSSGP_IE_IMSI), TLVP_LEN(tp, BSSGP_IE_IMSI),
+ true);
+ if (rc < 0 || mi->type != GSM_MI_TYPE_IMSI) {
+ LOGP(DBSSGP, LOGL_ERROR, "Invalid IMSI Mobile Identity\n");
+ return BSSGP_CAUSE_COND_IE_ERR;
+ }
} else {
LOGP(DBSSGP, LOGL_ERROR, "Neither TMSI IE nor IMSI IE is present\n");
return BSSGP_CAUSE_MISSING_COND_IE;
@@ -203,23 +203,21 @@
static int gprs_bssgp_pcu_rx_paging_cs(struct msgb *msg, struct tlv_parsed *tp)
{
- const uint8_t *mi;
- uint8_t mi_len;
+ struct osmo_mobile_identity mi;
int rc;
uint8_t *chan_needed = (uint8_t *)TLVP_VAL(tp, BSSGP_IE_CHAN_NEEDED);
- if ((rc = get_paging_mi(&mi, &mi_len, tp)) > 0)
+ if ((rc = get_paging_mi(&mi, tp)) > 0)
return bssgp_tx_status((enum gprs_bssgp_cause) rc, NULL, msg);
- return BTS::main_bts()->add_paging(chan_needed ? *chan_needed : 0, mi, mi_len);
+ return BTS::main_bts()->add_paging(chan_needed ? *chan_needed : 0, &mi);
}
static int gprs_bssgp_pcu_rx_paging_ps(struct msgb *msg, struct tlv_parsed *tp)
{
struct osmo_mobile_identity mi_imsi;
+ struct osmo_mobile_identity paging_mi;
uint16_t pgroup;
- const uint8_t *mi;
- uint8_t mi_len;
int rc;
if (!TLVP_PRESENT(tp, BSSGP_IE_IMSI)) {
@@ -238,10 +236,10 @@
return bssgp_tx_status(BSSGP_CAUSE_INV_MAND_INF, NULL, msg);
}
- if ((rc = get_paging_mi(&mi, &mi_len, tp)) > 0)
+ if ((rc = get_paging_mi(&paging_mi, tp)) > 0)
return bssgp_tx_status((enum gprs_bssgp_cause) rc, NULL, msg);
- return gprs_rlcmac_paging_request(mi, mi_len, pgroup);
+ return gprs_rlcmac_paging_request(&paging_mi, pgroup);
}
/* Receive a BSSGP PDU from a BSS on a PTP BVCI */
diff --git a/src/gprs_rlcmac.cpp b/src/gprs_rlcmac.cpp
index 879ea85..4fb75f7 100644
--- a/src/gprs_rlcmac.cpp
+++ b/src/gprs_rlcmac.cpp
@@ -32,18 +32,20 @@
extern void *tall_pcu_ctx;
-int gprs_rlcmac_paging_request(const uint8_t *mi, uint8_t mi_len, uint16_t pgroup)
+int gprs_rlcmac_paging_request(const struct osmo_mobile_identity *mi, uint16_t pgroup)
{
if (log_check_level(DRLCMAC, LOGL_NOTICE)) {
- struct osmo_mobile_identity omi = {};
char str[64];
- osmo_mobile_identity_decode(&omi, mi, mi_len, true);
- osmo_mobile_identity_to_str_buf(str, sizeof(str), &omi);
+ osmo_mobile_identity_to_str_buf(str, sizeof(str), mi);
LOGP(DRLCMAC, LOGL_NOTICE, "TX: [PCU -> BTS] Paging Request (CCCH) MI=%s\n", str);
}
bitvec *paging_request = bitvec_alloc(22, tall_pcu_ctx);
bitvec_unhex(paging_request, DUMMY_VEC);
- int plen = Encoding::write_paging_request(paging_request, mi, mi_len);
+ int plen = Encoding::write_paging_request(paging_request, mi);
+ if (plen <= 0) {
+ LOGP(DRLCMAC, LOGL_ERROR, "TX: [PCU -> BTS] Failed to encode Paging Request\n");
+ return -1;
+ }
pcu_l1if_tx_pch(paging_request, plen, pgroup);
bitvec_free(paging_request);
diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h
index 789b8db..6587c40 100644
--- a/src/gprs_rlcmac.h
+++ b/src/gprs_rlcmac.h
@@ -93,7 +93,7 @@
int gprs_rlcmac_tx_ul_ud(gprs_rlcmac_tbf *tbf);
-int gprs_rlcmac_paging_request(const uint8_t *mi, uint8_t mi_len, uint16_t pgroup);
+int gprs_rlcmac_paging_request(const struct osmo_mobile_identity *mi, uint16_t pgroup);
struct msgb *gprs_rlcmac_app_info_msg(const struct gsm_pcu_if_app_info_req *req);
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index 7fa82fb..0712470 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -677,6 +677,9 @@
static int pcu_rx_pag_req(struct gsm_pcu_if_pag_req *pag_req)
{
+ struct osmo_mobile_identity mi;
+ int rc;
+
LOGP(DL1IF, LOGL_DEBUG, "Paging request received: chan_needed=%d "
"length=%d\n", pag_req->chan_needed, pag_req->identity_lv[0]);
@@ -687,8 +690,13 @@
return -EINVAL;
}
- return BTS::main_bts()->add_paging(pag_req->chan_needed, &pag_req->identity_lv[1],
- pag_req->identity_lv[0]);
+ rc = osmo_mobile_identity_decode(&mi, &pag_req->identity_lv[1], pag_req->identity_lv[0], true);
+ if (rc < 0) {
+ LOGP(DL1IF, LOGL_ERROR, "Failed to decode Mobile Identity in Paging Request (rc=%d)\n", rc);
+ return -EINVAL;
+ }
+
+ return BTS::main_bts()->add_paging(pag_req->chan_needed, &mi);
}
static int pcu_rx_susp_req(struct gsm_pcu_if_susp_req *susp_req)
diff --git a/src/pdch.cpp b/src/pdch.cpp
index af84724..fbbeddc 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -266,15 +266,21 @@
return NULL;
}
-bool gprs_rlcmac_pdch::add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len)
+bool gprs_rlcmac_pdch::add_paging(uint8_t chan_needed, const struct osmo_mobile_identity *mi)
{
+ int rc;
struct gprs_rlcmac_paging *pag = talloc_zero(tall_pcu_ctx, struct gprs_rlcmac_paging);
if (!pag)
return false;
pag->chan_needed = chan_needed;
- pag->identity_lv[0] = mi_len;
- memcpy(&pag->identity_lv[1], mi, mi_len);
+ rc = osmo_mobile_identity_encode_buf(pag->identity_lv + 1, sizeof(pag->identity_lv) - 1, mi, true);
+ if (rc <= 0) {
+ LOGP(DRLCMAC, LOGL_ERROR, "Cannot encode Mobile Identity (rc=%d)\n", rc);
+ talloc_free(pag);
+ return false;
+ }
+ pag->identity_lv[0] = rc;
llist_add(&pag->list, &paging_list);
diff --git a/src/pdch.h b/src/pdch.h
index ec35174..e1e3688 100644
--- a/src/pdch.h
+++ b/src/pdch.h
@@ -48,7 +48,7 @@
struct gprs_rlcmac_paging *dequeue_paging();
struct msgb *packet_paging_request();
- bool add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len);
+ bool add_paging(uint8_t chan_needed, const struct osmo_mobile_identity *mi);
void free_resources();
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/19768
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Iad845acb0096b75dc453105c9c16b2252879b4ca
Gerrit-Change-Number: 19768
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200824/9b765435/attachment.htm>