Attention is currently required from: dexter.
laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38160?usp=email )
Change subject: pySim-prog: fix sourcecode formatting
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
not quite sure this is a fix, but no need to argue about it.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/38160?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I0e567a1a681891f33f170c5df50c691b20b6a978
Gerrit-Change-Number: 38160
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 16 Sep 2024 10:48:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/38156?usp=email )
Change subject: llc: Mark old/current tlli as all 1's when unassigning LLME
......................................................................
llc: Mark old/current tlli as all 1's when unassigning LLME
TS 44.064 section 8.3.3 (and other sections) talk about special
unassigned value of "all 1's", but I couldn't find any reference to a
"all 0's" specific value/meaning.
In practice in the code this may not be super important since those
values may not ve checked due to the FSM state, but in any case they are
initially set to all 1's, so it makes total sense to re-set them to the
same unassigned value instead of a randomly chosen all 0's value.
Change-Id: I660c8d0ef08b34f8cb74fd51b5c59e5628d687ae
(cherry picked from commit 35c178e84d25a216a5c2d9bd1320d072e36937fa)
---
M src/sgsn/gprs_llc.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
diff --git a/src/sgsn/gprs_llc.c b/src/sgsn/gprs_llc.c
index 6f56385..82e876d 100644
--- a/src/sgsn/gprs_llc.c
+++ b/src/sgsn/gprs_llc.c
@@ -1112,7 +1112,7 @@
llme->state = GPRS_LLMS_ASSIGNED;
} else if (old_tlli != TLLI_UNASSIGNED && new_tlli == TLLI_UNASSIGNED) {
/* TLLI Unassignment 8.3.3) */
- llme->tlli = llme->old_tlli = 0;
+ llme->tlli = llme->old_tlli = TLLI_UNASSIGNED;
llme->state = GPRS_LLMS_UNASSIGNED;
for (i = 0; i < ARRAY_SIZE(llme->lle); i++) {
struct gprs_llc_lle *l = &llme->lle[i];
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/38156?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-sgsn
Gerrit-Branch: osmith/1.12.1
Gerrit-Change-Id: I660c8d0ef08b34f8cb74fd51b5c59e5628d687ae
Gerrit-Change-Number: 38156
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/38154?usp=email )
Change subject: Fix DeactPDPCtxAcc when UE goes PMM ENABLED but lost its PDP context
......................................................................
Fix DeactPDPCtxAcc when UE goes PMM ENABLED but lost its PDP context
Scenario: UE activates a PDP context, then after a while goes PMM IDLE
(Iu conn is destroyed but PDP is kept).
When UE connects through Iu again, it sends eg. RAU or ServiceRequest
with pdp_status bitmask statis the active NSAPIs.
If some NSAPI (PDP context) is enabled at SGSN but doesn't show up in
the bitmask, SGSN will destroy the PDP context with GGSN
(DeletePDPContextReq) towards GGSN prior to re-creating it.
When SGSN receives the DeletePDPContextResp, it would forward a
DeactivatePDPContextReq to the UE for a PDP context which was not known
by the UE anymore, this is wrong.
With this patch, the state of the NSAPI/PDP at the UE side is tracked,
and used to know whether when the PDP gets deleted on the GGSN side then
it needs to also be deleted on the Iu side.
Change-Id: I0ccd9228d71c29248b5f510356dbfdb09565dc30
(cherry picked from commit 4ced617eb6bb6608b6fcc14f359366551e636923)
---
M include/osmocom/sgsn/pdpctx.h
M src/sgsn/gprs_gmm.c
M src/sgsn/sgsn_libgtp.c
3 files changed, 8 insertions(+), 3 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
diff --git a/include/osmocom/sgsn/pdpctx.h b/include/osmocom/sgsn/pdpctx.h
index 255a77d..f3cf0ed 100644
--- a/include/osmocom/sgsn/pdpctx.h
+++ b/include/osmocom/sgsn/pdpctx.h
@@ -68,6 +68,7 @@
//uint32_t qos_profile_neg;
uint8_t radio_prio;
//uint32_t charging_id;
+ bool ue_pdp_active; /* PDP Context is active for this NSAPI? */
struct osmo_timer_list timer;
unsigned int T; /* Txxxx number */
diff --git a/src/sgsn/gprs_gmm.c b/src/sgsn/gprs_gmm.c
index 653c425..ab8b7ef 100644
--- a/src/sgsn/gprs_gmm.c
+++ b/src/sgsn/gprs_gmm.c
@@ -1591,6 +1591,7 @@
LOGMMCTXP(LOGL_NOTICE, mmctx, "Dropping PDP context for NSAPI=%u "
"due to PDP CTX STATUS IE=0x%02x%02x\n",
pdp->nsapi, pdp_status[1], pdp_status[0]);
+ pdp->ue_pdp_active = false;
if (pdp->ggsn)
sgsn_delete_pdp_ctx(pdp);
else /* GTP side already detached, freeing */
diff --git a/src/sgsn/sgsn_libgtp.c b/src/sgsn/sgsn_libgtp.c
index 9edd0c6..4885ada 100644
--- a/src/sgsn/sgsn_libgtp.c
+++ b/src/sgsn/sgsn_libgtp.c
@@ -378,6 +378,7 @@
rc = gsm48_tx_gsm_act_pdp_acc(pctx);
if (rc < 0)
return rc;
+ pctx->ue_pdp_active = true;
if (pctx->mm->ran_type == MM_CTX_T_GERAN_Gb) {
/* Send SNDCP XID to MS */
@@ -567,9 +568,11 @@
return -ENOTSUP;
#endif
}
-
- /* Confirm deactivation of PDP context to MS */
- rc = gsm48_tx_gsm_deact_pdp_acc(pctx);
+ if (pctx->ue_pdp_active) {
+ /* Confirm deactivation of PDP context to MS */
+ rc = gsm48_tx_gsm_deact_pdp_acc(pctx);
+ pctx->ue_pdp_active = false;
+ }
} else {
LOGPDPCTXP(LOGL_NOTICE, pctx,
"Not deactivating SNDCP layer since the MM context "
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/38154?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-sgsn
Gerrit-Branch: osmith/1.12.1
Gerrit-Change-Id: I0ccd9228d71c29248b5f510356dbfdb09565dc30
Gerrit-Change-Number: 38154
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/38155?usp=email )
Change subject: Fix double free during RAU with unexpected Old RAI
......................................................................
Fix double free during RAU with unexpected Old RAI
If an MS which had an MMCTX at the SGSN sent RAU update with an
unexpected Old RA field, the RAU was rejected and LLME (LLC layer)
unassigned (freed), because no MMCTX was found matching the wrong old
RA.
However, an MMCTX may actually exist pointing to that LLME, and hence
when the LLME is freed, it stayed unnoticed with a dangling pointer to
the freed LLME in ctx->gb.llme.
Let's try to harder to avoid this kind of bugs which make osmo-sgsn
crash.
Once we properly split the code into separate independent layers (LLC,
MMCTX, etc.) each holding their own structs, this kind of bugs shouldn't
happen anymore.
Related: OS#6441
Change-Id: I5a4328c6e945b85dd815215724feecadba59c435
(cherry picked from commit 868d818e6e9c39fcd4acb362708dc162cb4ee7f4)
---
M include/osmocom/sgsn/mmctx.h
M src/sgsn/gprs_gmm.c
M src/sgsn/mmctx.c
3 files changed, 31 insertions(+), 0 deletions(-)
Approvals:
pespin: Looks good to me, approved
osmith: Verified
diff --git a/include/osmocom/sgsn/mmctx.h b/include/osmocom/sgsn/mmctx.h
index c19f599..5e4662f 100644
--- a/include/osmocom/sgsn/mmctx.h
+++ b/include/osmocom/sgsn/mmctx.h
@@ -257,6 +257,7 @@
struct sgsn_mm_ctx *sgsn_mm_ctx_by_ptmsi(uint32_t tmsi);
struct sgsn_mm_ctx *sgsn_mm_ctx_by_imsi(const char *imsi);
struct sgsn_mm_ctx *sgsn_mm_ctx_by_ue_ctx(const void *uectx);
+struct sgsn_mm_ctx *sgsn_mm_ctx_by_llme(const struct gprs_llc_llme *llme);
/* look-up by matching TLLI and P-TMSI (think twice before using this) */
struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli_and_ptmsi(uint32_t tlli,
diff --git a/src/sgsn/gprs_gmm.c b/src/sgsn/gprs_gmm.c
index ab8b7ef..911d42d 100644
--- a/src/sgsn/gprs_gmm.c
+++ b/src/sgsn/gprs_gmm.c
@@ -1748,6 +1748,21 @@
* in the MS */
LOGGBP(llme, DMM, LOGL_NOTICE, "LLC XID RESET\n");
gprs_llgmm_reset_oldmsg(msg, GPRS_SAPI_GMM, llme);
+
+ /* The RAU didn't come from expected TLLI+RAI, so it's for sure bad and should be rejected.
+ * In any case, before unassigning (freeing) the LLME during the REJECT below, make sure
+ * beforehand that if there's an mmctx relating to that llme it is also freed.
+ * Otherwise it would be kept pointining at a dangling freed llme object.
+ */
+ mmctx = sgsn_mm_ctx_by_llme(llme);
+ if (mmctx) {
+ char old_ra_id_name[32];
+ osmo_rai_name_buf(old_ra_id_name, sizeof(old_ra_id_name), &old_ra_id);
+ LOGMMCTXP(LOGL_NOTICE, mmctx,
+ "Rx RA Update Request with unexpected TLLI=%08x Old RA=%s (expected Old RA: %s)!\n",
+ msgb_tlli(msg), old_ra_id_name, osmo_rai_name(&mmctx->ra));
+ /* mmctx will be released (and its llme un assigned) after REJECT below. */
+ }
}
/* The MS has to perform GPRS attach */
/* Device is still IMSI attached for CS but initiate GPRS ATTACH,
diff --git a/src/sgsn/mmctx.c b/src/sgsn/mmctx.c
index 0e93092..459f6cf 100644
--- a/src/sgsn/mmctx.c
+++ b/src/sgsn/mmctx.c
@@ -105,6 +105,21 @@
return NULL;
}
+
+/* look-up an SGSN MM context based on Gb LLME context (struct gprs_llc_llme)*/
+struct sgsn_mm_ctx *sgsn_mm_ctx_by_llme(const struct gprs_llc_llme *llme)
+{
+ struct sgsn_mm_ctx *ctx;
+
+ llist_for_each_entry (ctx, &sgsn->mm_list, list) {
+ if (ctx->ran_type == MM_CTX_T_GERAN_Gb
+ && llme == ctx->gb.llme)
+ return ctx;
+ }
+
+ return NULL;
+}
+
/* look-up a SGSN MM context based on TLLI + RAI */
struct sgsn_mm_ctx *sgsn_mm_ctx_by_tlli(uint32_t tlli,
const struct gprs_ra_id *raid)
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/38155?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-sgsn
Gerrit-Branch: osmith/1.12.1
Gerrit-Change-Id: I5a4328c6e945b85dd815215724feecadba59c435
Gerrit-Change-Number: 38155
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>