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
This commit is contained in:
Neels Hofmeyr 2020-08-21 16:21:23 +02:00
parent 0052051c07
commit 59fc0bda6e
10 changed files with 58 additions and 40 deletions

View File

@ -355,7 +355,7 @@ void BTS::set_current_block_frame_number(int fn, unsigned max_delay)
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 @@ int BTS::add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len)
};
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 @@ int BTS::add_paging(uint8_t chan_needed, const uint8_t *mi, uint8_t mi_len)
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);

View File

@ -309,7 +309,7 @@ public:
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);

View File

@ -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 @@ void Encoding::write_packet_downlink_assignment(RlcMacDownlink_t * block,
}
/* 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 @@ int Encoding::write_paging_request(bitvec * dest, const uint8_t *mi, uint8_t mi_
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);

View File

@ -78,7 +78,7 @@ public:
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);

View File

@ -171,11 +171,8 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
}
/* 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 @@ static unsigned int get_paging_mi(const uint8_t **mi, uint8_t *mi_len,
}
/* 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 unsigned int get_paging_mi(const uint8_t **mi, uint8_t *mi_len,
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 @@ static int gprs_bssgp_pcu_rx_paging_ps(struct msgb *msg, struct tlv_parsed *tp)
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 */

View File

@ -32,18 +32,20 @@ extern "C" {
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);

View File

@ -93,7 +93,7 @@ enum gprs_rlcmac_block_type {
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);

View File

@ -677,6 +677,9 @@ static int pcu_rx_time_ind(struct gsm_pcu_if_time_ind *time_ind)
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 @@ static int pcu_rx_pag_req(struct gsm_pcu_if_pag_req *pag_req)
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)

View File

@ -266,15 +266,21 @@ free_ret:
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);

View File

@ -48,7 +48,7 @@ struct gprs_rlcmac_pdch {
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();