sgsn: Don't allow mmctx == NULL in sgsn_update_subscriber_data
Currently, sgsn_update_subscriber_data can be called with mmctx == NULL and will find and associate the right context (if present) based on the subscriber's IMSI. This will not happen in regular use any more, since sgsn_update_subscriber_data will only be called when subscribers are used (auth mode 'remote') and in this case gprs_subscr_get_or_create_by_mmctx will already be called by sgsn_auth_request. Therefore, MM context and subscriber are always associated except for some test cases and experimental VTY usage. The current implementation of sgsn_update_subscriber_data also causes additional complexity for the deletion on MM contexts to avoid a ipossible double-free MM contexts. This commit removes the MM context <-> subscriber association code from sgsn_update_subscriber_data. That function must always be called with mmctx != NULL, now. To avoid problems with VTY and test usage, the calling subscriber function now only call sgsn_update_subscriber_data when mmctx != NULL, since the purpose of that function is to update that state of an existing MM context after subscriber data has been changed. Sponsored-by: On-Waves ehf
This commit is contained in:
parent
925c57fb54
commit
555b2e5ac1
|
@ -339,8 +339,7 @@ int gprs_subscr_query_auth_info(struct gsm_subscriber *subscr);
|
|||
int gprs_subscr_location_update(struct gsm_subscriber *subscr);
|
||||
|
||||
/* Called on subscriber data updates */
|
||||
void sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
|
||||
struct gsm_subscriber *subscr);
|
||||
void sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx);
|
||||
|
||||
int gprs_sndcp_vty_init(void);
|
||||
struct sgsn_instance;
|
||||
|
|
|
@ -487,28 +487,11 @@ int sgsn_force_reattach_oldmsg(struct msgb *oldmsg)
|
|||
return gsm0408_gprs_force_reattach_oldmsg(oldmsg);
|
||||
}
|
||||
|
||||
void sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
|
||||
struct gsm_subscriber *subscr)
|
||||
void sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx)
|
||||
{
|
||||
if (!mmctx && subscr && strlen(subscr->imsi) > 0) {
|
||||
mmctx = sgsn_mm_ctx_by_imsi(subscr->imsi);
|
||||
OSMO_ASSERT(!mmctx || !mmctx->subscr || mmctx->subscr == subscr);
|
||||
}
|
||||
|
||||
if (!mmctx) {
|
||||
LOGP(DMM, LOGL_INFO,
|
||||
"Subscriber data update for unregistered MM context, IMSI %s\n",
|
||||
subscr->imsi);
|
||||
return;
|
||||
}
|
||||
|
||||
OSMO_ASSERT(mmctx != NULL);
|
||||
LOGMMCTXP(LOGL_INFO, mmctx, "Subscriber data update\n");
|
||||
|
||||
if (!subscr->sgsn_data->mm && !mmctx->subscr) {
|
||||
mmctx->subscr = subscr_get(subscr);
|
||||
mmctx->subscr->sgsn_data->mm = mmctx;
|
||||
}
|
||||
|
||||
sgsn_auth_update(mmctx);
|
||||
}
|
||||
|
||||
|
|
|
@ -614,7 +614,8 @@ void gprs_subscr_update(struct gsm_subscriber *subscr)
|
|||
subscr->flags &= ~GPRS_SUBSCRIBER_UPDATE_LOCATION_PENDING;
|
||||
subscr->flags &= ~GSM_SUBSCRIBER_FIRST_CONTACT;
|
||||
|
||||
sgsn_update_subscriber_data(subscr->sgsn_data->mm, subscr);
|
||||
if (subscr->sgsn_data->mm)
|
||||
sgsn_update_subscriber_data(subscr->sgsn_data->mm);
|
||||
}
|
||||
|
||||
void gprs_subscr_update_auth_info(struct gsm_subscriber *subscr)
|
||||
|
@ -625,7 +626,8 @@ void gprs_subscr_update_auth_info(struct gsm_subscriber *subscr)
|
|||
subscr->flags &= ~GPRS_SUBSCRIBER_UPDATE_AUTH_INFO_PENDING;
|
||||
subscr->flags &= ~GSM_SUBSCRIBER_FIRST_CONTACT;
|
||||
|
||||
sgsn_update_subscriber_data(subscr->sgsn_data->mm, subscr);
|
||||
if (subscr->sgsn_data->mm)
|
||||
sgsn_update_subscriber_data(subscr->sgsn_data->mm);
|
||||
}
|
||||
|
||||
struct gsm_subscriber *gprs_subscr_get_or_create_by_mmctx(struct sgsn_mm_ctx *mmctx)
|
||||
|
|
|
@ -61,14 +61,13 @@ int bssgp_tx_dl_ud(struct msgb *msg, uint16_t pdu_lifetime,
|
|||
}
|
||||
|
||||
/* override, requires '-Wl,--wrap=sgsn_update_subscriber_data' */
|
||||
void __real_sgsn_update_subscriber_data(struct sgsn_mm_ctx *, struct gsm_subscriber *);
|
||||
void (*update_subscriber_data_cb)(struct sgsn_mm_ctx *, struct gsm_subscriber *) =
|
||||
void __real_sgsn_update_subscriber_data(struct sgsn_mm_ctx *);
|
||||
void (*update_subscriber_data_cb)(struct sgsn_mm_ctx *) =
|
||||
&__real_sgsn_update_subscriber_data;
|
||||
|
||||
void __wrap_sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
|
||||
struct gsm_subscriber *subscr)
|
||||
void __wrap_sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx)
|
||||
{
|
||||
(*update_subscriber_data_cb)(mmctx, subscr);
|
||||
(*update_subscriber_data_cb)(mmctx);
|
||||
}
|
||||
|
||||
/* override, requires '-Wl,--wrap=gprs_subscr_request_update_location' */
|
||||
|
@ -191,12 +190,12 @@ static void test_llme(void)
|
|||
}
|
||||
|
||||
struct gsm_subscriber *last_updated_subscr = NULL;
|
||||
void my_dummy_sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx,
|
||||
struct gsm_subscriber *subscr)
|
||||
void my_dummy_sgsn_update_subscriber_data(struct sgsn_mm_ctx *mmctx)
|
||||
{
|
||||
OSMO_ASSERT(mmctx);
|
||||
fprintf(stderr, "Called %s, mmctx = %p, subscr = %p\n",
|
||||
__func__, mmctx, subscr);
|
||||
last_updated_subscr = subscr;
|
||||
__func__, mmctx, mmctx->subscr);
|
||||
last_updated_subscr = mmctx->subscr;
|
||||
}
|
||||
|
||||
static void assert_subscr(const struct gsm_subscriber *subscr, const char *imsi)
|
||||
|
@ -266,7 +265,9 @@ static void test_subscriber(void)
|
|||
/* Update entry 1 */
|
||||
last_updated_subscr = NULL;
|
||||
gprs_subscr_update(s1);
|
||||
OSMO_ASSERT(last_updated_subscr == s1);
|
||||
OSMO_ASSERT(last_updated_subscr == NULL);
|
||||
OSMO_ASSERT(s1->sgsn_data->mm == NULL);
|
||||
OSMO_ASSERT((s1->flags & GSM_SUBSCRIBER_FIRST_CONTACT) == 0);
|
||||
|
||||
/* There is no subscriber cache. Verify it */
|
||||
gprs_subscr_cleanup(s1);
|
||||
|
|
Loading…
Reference in New Issue