Project

General

Profile

Bug #4339

null deref in gsm48_rx_gmm_ra_upd_req() / bssgp_parse_cell_id() / gsm48_parse_ra()

Added by neels 10 months ago. Updated 6 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
12/26/2019
Due date:
% Done:

100%

Spec Reference:
Tags:

Description

#0  gsm48_parse_ra (raid=0x5651660dd19c, buf=buf@entry=0x0) at ../../../../src/libosmocore/src/gsm/gsm48.c:788
788        raid->mcc = (buf[0] & 0xf) * 100;
(gdb) bt
#0  gsm48_parse_ra (raid=0x5651660dd19c, buf=buf@entry=0x0) at ../../../../src/libosmocore/src/gsm/gsm48.c:788
#1  0x00007f51d7686e49 in bssgp_parse_cell_id (raid=<optimized out>, buf=0x0) at ../../../../src/libosmocore/src/gb/gprs_bssgp.c:252
#2  0x000056516543b08d in gsm48_rx_gmm_ra_upd_req (mmctx=0x5651660dd140, msg=0x5651660e1cc0, llme=0x0) at ../../../../src/osmo-sgsn/src/sgsn/gprs_gmm.c:1646
#3  0x00007f51d7080e81 in talloc_named_const () from /usr/lib/x86_64-linux-gnu/libtalloc.so.2

hotfix would be

--- a/src/sgsn/gprs_gmm.c
+++ b/src/sgsn/gprs_gmm.c
@@ -1642,7 +1642,7 @@ static int gsm48_rx_gmm_ra_upd_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg,
        rate_ctr_inc(&mmctx->ctrg->ctr[GMM_CTR_PKTS_SIG_IN]);

        /* Update the MM context with the new RA-ID */
-       if (mmctx->ran_type == MM_CTX_T_GERAN_Gb) {
+       if (mmctx->ran_type == MM_CTX_T_GERAN_Gb && msgb_bcid(msg)) {
                bssgp_parse_cell_id(&mmctx->ra, msgb_bcid(msg));
                /* Update the MM context with the new (i.e. foreign) TLLI */
                mmctx->gb.tlli = msgb_tlli(msg);

but not sure what the root cause is (lynxis?)

History

#1 Updated by laforge 10 months ago

  • Status changed from New to In Progress
  • Assignee set to neels
  • % Done changed from 0 to 60

proposed patch by Neels in https://gerrit.osmocom.org/c/osmo-sgsn/+/16744/1

SGSN_Tests_Iu.TC_geran_attach_iu_rau tests exactly for this scenario: A Subscriber / MM context that is so far attached via GERAN, but now receives a RAU via UTRAN/Iu.

#2 Updated by neels 6 months ago

lynxis has indicated various times that he has a proper fix for this,
my patch is written with very shallow knowledge of what is really going wrong there.

#3 Updated by neels 6 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 60 to 100

above mentioned test passes even though the patch was never merged.
Apparently we have merged another fix for that crash?
Anyway, the linked patch cannot be harmful and still builds, so merging it anyway.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)