Review at https://gerrit.osmocom.org/171
gprs_gmm.c: Don't try to de-reference NULL mmctx
There was a comment in the code that certain GMM messages require a
valid mmctx pointer. However, nothing actually checked if that pointer
was in fact non-NULL. We plainly crashed if a MS would send us the
wrong message in the wrong state.
Original patch by Harald Welte, but it broke message validity checking,
resulting in sgsn_test failure. This re-implements the NULL check in a
different way, as explained by in-code comment.
Change-Id: I7908de65bec91599f7042549b832cbbd7ae5a9a8
---
M openbsc/src/gprs/gprs_gmm.c
1 file changed, 32 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/71/171/1
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 1e10701..e788e3d 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1339,6 +1339,15 @@
return rc;
}
+ /*
+ * For a few messages, mmctx may be NULL. For most, we want to ensure a
+ * non-NULL mmctx. At the same time, we want to keep the message
+ * validity check intact, so that all message types appear in the
+ * switch statement and the default case thus means "unknown message".
+ * If we split the switch in two parts to check non-NULL halfway, the
+ * unknown-message check breaks, or we'd need to duplicate the switch
+ * cases in both parts. Just keep one large switch and add some gotos.
+ */
switch (gh->msg_type) {
case GSM48_MT_GMM_RA_UPD_REQ:
rc = gsm48_rx_gmm_ra_upd_req(mmctx, msg, llme);
@@ -1348,20 +1357,30 @@
break;
/* For all the following types mmctx can not be NULL */
case GSM48_MT_GMM_ID_RESP:
+ if (!mmctx)
+ goto null_mmctx;
rc = gsm48_rx_gmm_id_resp(mmctx, msg);
break;
case GSM48_MT_GMM_STATUS:
+ if (!mmctx)
+ goto null_mmctx;
rc = gsm48_rx_gmm_status(mmctx, msg);
break;
case GSM48_MT_GMM_DETACH_REQ:
+ if (!mmctx)
+ goto null_mmctx;
rc = gsm48_rx_gmm_det_req(mmctx, msg);
break;
case GSM48_MT_GMM_DETACH_ACK:
+ if (!mmctx)
+ goto null_mmctx;
LOGMMCTXP(LOGL_INFO, mmctx, "-> DETACH ACK\n");
mm_ctx_cleanup_free(mmctx, "GPRS DETACH ACK");
rc = 0;
break;
case GSM48_MT_GMM_ATTACH_COMPL:
+ if (!mmctx)
+ goto null_mmctx;
/* only in case SGSN offered new P-TMSI */
LOGMMCTXP(LOGL_INFO, mmctx, "-> ATTACH COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
@@ -1380,6 +1399,8 @@
osmo_signal_dispatch(SS_SGSN, S_SGSN_ATTACH, &sig_data);
break;
case GSM48_MT_GMM_RA_UPD_COMPL:
+ if (!mmctx)
+ goto null_mmctx;
/* only in case SGSN offered new P-TMSI */
LOGMMCTXP(LOGL_INFO, mmctx, "-> ROUTING AREA UPDATE COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
@@ -1398,6 +1419,8 @@
osmo_signal_dispatch(SS_SGSN, S_SGSN_UPDATE, &sig_data);
break;
case GSM48_MT_GMM_PTMSI_REALL_COMPL:
+ if (!mmctx)
+ goto null_mmctx;
LOGMMCTXP(LOGL_INFO, mmctx, "-> PTMSI REALLLICATION COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
mmctx->t3350_mode = GMM_T3350_MODE_NONE;
@@ -1409,6 +1432,8 @@
rc = 0;
break;
case GSM48_MT_GMM_AUTH_CIPH_RESP:
+ if (!mmctx)
+ goto null_mmctx;
rc = gsm48_rx_gmm_auth_ciph_resp(mmctx, msg);
break;
default:
@@ -1419,6 +1444,13 @@
}
return rc;
+
+null_mmctx:
+ LOGP(DMM, LOGL_ERROR,
+ "Received GSM 04.08 message type 0x%02x,"
+ " but no MM context available\n",
+ gh->msg_type);
+ return -EINVAL;
}
static void mmctx_timer_cb(void *_mm)
--
To view, visit https://gerrit.osmocom.org/171
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7908de65bec91599f7042549b832cbbd7ae5a9a8
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr(a)sysmocom.de>
Review at https://gerrit.osmocom.org/184
dyn PDCH: Add new_lchan argument to bsc_handover_start()
This is useful if the caller already allocated a new lchan, which will be used
to dynamically re-assign lchans.
The old behavior is maintained by passing NULL.
Change-Id: I2b7151f32f0c04c22f294eb5dd3c7d7dfddf35e7
---
M openbsc/include/openbsc/handover.h
M openbsc/src/libbsc/handover_decision.c
M openbsc/src/libbsc/handover_logic.c
M openbsc/src/libmsc/vty_interface_layer3.c
4 files changed, 15 insertions(+), 10 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/84/184/1
diff --git a/openbsc/include/openbsc/handover.h b/openbsc/include/openbsc/handover.h
index 3fe71a2..a4844c5 100644
--- a/openbsc/include/openbsc/handover.h
+++ b/openbsc/include/openbsc/handover.h
@@ -3,7 +3,8 @@
struct gsm_subscriber_connection;
-int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_bts *bts);
+int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan,
+ struct gsm_bts *bts);
/* clear any operation for this connection */
void bsc_clear_handover(struct gsm_subscriber_connection *conn, int free_lchan);
diff --git a/openbsc/src/libbsc/handover_decision.c b/openbsc/src/libbsc/handover_decision.c
index 0f07bca..8b92177 100644
--- a/openbsc/src/libbsc/handover_decision.c
+++ b/openbsc/src/libbsc/handover_decision.c
@@ -48,7 +48,7 @@
}
/* and actually try to handover to that cell */
- return bsc_handover_start(lchan, new_bts);
+ return bsc_handover_start(lchan, NULL, new_bts);
}
/* did we get a RXLEV for a given cell in the given report? */
diff --git a/openbsc/src/libbsc/handover_logic.c b/openbsc/src/libbsc/handover_logic.c
index 641cee4..3e38fda 100644
--- a/openbsc/src/libbsc/handover_logic.c
+++ b/openbsc/src/libbsc/handover_logic.c
@@ -87,10 +87,13 @@
/* Hand over the specified logical channel to the specified new BTS. This is
* the main entry point for the actual handover algorithm, after the decision
- * whether to initiate HO to a specific BTS. */
-int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_bts *bts)
+ * whether to initiate HO to a specific BTS.
+ *
+ * If new_lchan is NULL, allocate a new lchan. If not NULL, new_lchan must be a
+ * newly allocated lchan passed in by the caller. */
+int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_lchan *new_lchan,
+ struct gsm_bts *new_bts)
{
- struct gsm_lchan *new_lchan;
struct bsc_handover *ho;
static uint8_t ho_ref;
int rc;
@@ -101,19 +104,20 @@
return -EBUSY;
DEBUGP(DHO, "(old_lchan on BTS %u, new BTS %u)\n",
- old_lchan->ts->trx->bts->nr, bts->nr);
+ old_lchan->ts->trx->bts->nr, new_bts->nr);
- osmo_counter_inc(bts->network->stats.handover.attempted);
+ osmo_counter_inc(new_bts->network->stats.handover.attempted);
if (!old_lchan->conn) {
LOGP(DHO, LOGL_ERROR, "Old lchan lacks connection data.\n");
return -ENOSPC;
}
- new_lchan = lchan_alloc(bts, old_lchan->type, 0);
+ if (!new_lchan)
+ new_lchan = lchan_alloc(new_bts, old_lchan->type, 0);
if (!new_lchan) {
LOGP(DHO, LOGL_NOTICE, "No free channel\n");
- osmo_counter_inc(bts->network->stats.handover.no_channel);
+ osmo_counter_inc(new_bts->network->stats.handover.no_channel);
return -ENOSPC;
}
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c
index 5d74e04..2ad7eab 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
@@ -641,7 +641,7 @@
}
/* now start the handover */
- ret = bsc_handover_start(conn->lchan, bts);
+ ret = bsc_handover_start(conn->lchan, NULL, bts);
if (ret != 0) {
vty_out(vty, "%% Handover failed with errno %d.%s",
ret, VTY_NEWLINE);
--
To view, visit https://gerrit.osmocom.org/184
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b7151f32f0c04c22f294eb5dd3c7d7dfddf35e7
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger(a)freyther.de>
Review at https://gerrit.osmocom.org/183
comment tweak for bsc_handover_start()
Have a comment only in the .c file to remove dup, tweak wording.
Change-Id: I6d19e2b5a794f8b5d8fb71791719447362c5ce85
---
M openbsc/include/openbsc/handover.h
M openbsc/src/libbsc/handover_logic.c
2 files changed, 3 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/83/183/1
diff --git a/openbsc/include/openbsc/handover.h b/openbsc/include/openbsc/handover.h
index bd0d8ad..3fe71a2 100644
--- a/openbsc/include/openbsc/handover.h
+++ b/openbsc/include/openbsc/handover.h
@@ -3,9 +3,6 @@
struct gsm_subscriber_connection;
-/* Hand over the specified logical channel to the specified new BTS.
- * This is the main entry point for the actual handover algorithm,
- * after it has decided it wants to initiate HO to a specific BTS */
int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_bts *bts);
/* clear any operation for this connection */
diff --git a/openbsc/src/libbsc/handover_logic.c b/openbsc/src/libbsc/handover_logic.c
index 2b8c386..641cee4 100644
--- a/openbsc/src/libbsc/handover_logic.c
+++ b/openbsc/src/libbsc/handover_logic.c
@@ -85,9 +85,9 @@
return NULL;
}
-/* Hand over the specified logical channel to the specified new BTS.
- * This is the main entry point for the actual handover algorithm,
- * after it has decided it wants to initiate HO to a specific BTS */
+/* Hand over the specified logical channel to the specified new BTS. This is
+ * the main entry point for the actual handover algorithm, after the decision
+ * whether to initiate HO to a specific BTS. */
int bsc_handover_start(struct gsm_lchan *old_lchan, struct gsm_bts *bts)
{
struct gsm_lchan *new_lchan;
--
To view, visit https://gerrit.osmocom.org/183
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d19e2b5a794f8b5d8fb71791719447362c5ce85
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger(a)freyther.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr(a)sysmocom.de>
Hi Guys,
I am a bit fed-up to have to receive Neels non-productive mails every single day and given my tight work schedule let's have him win, declare Gerrit a failure and return to patchwork.
The twist is that I am no longer willing to clean behind everyone on the patchwork UI. So somebody else needs to sign-up for the patch merging and gardening in the WebUI.
My proposal would be:
* Sunday morning we change all ACLs (no more pushes, no more merges/submits/reviews)
* Luckily we have all patches in our patchwork and can continue from there.
agreed?
holger
Hi all,
I have just changed the access configuration of gerrit.osmocom.org:
* added a group of "known users"
* allow create+push to refs/heads/users/* to this group
* disallow access to refs/users/* (except to admins for cleanup)
* added a sysmocom group
* allow create+push to refs/heads/sysmocom/* to this group
Questions:
* allow global namespace access to "known users"?
* or move old user branches to users/?
Causality:
Previously, gerrit granted create+push access to refs/users/* (note: no
"heads") to anybody. However, branches pushed there were not being replicated
to git.osmocom.org. IMHO we should not have this fragmentation of repositories.
Instead, we could allow create+push to refs/heads/users/* (note: "heads") to
any registered user. The refs/heads/users/* namespace will replicate to
git.osmocom.org repositories automatically. But:
Before gerrit, anyone would be able to push to any branch. We relied on trust
that we wouldn't mess with other devs' branches or push to master without
review. That worked out pretty well. But, before gerrit, we would actively
enable users we trust. With gerrit, ANYONE can register without any project
member even noticing, and start pushing right away. Thus we should rather
limit the push access, e.g. to refs/heads/users/*
When granting push access to the users/* namespace to ANYONE, one problem
remains: any troll could commit any amount of completely unrelated data, and we
would readily replicate it to our "upstream" git.osmocom.org repositories.
So, actually, instead of allowing push access to users/* to anyone, I have
added a group "known users" to gerrit, which should typically contain anyone we
trust not to be a troll. I have added all gerrit users I know to this group.
Anyone who wishes to push to a users/* branch must request to be added to the
"known users" group. The threshold to join this group should be low.
(The ability to push patches for review is not affected.)
Since certain subgroups like to collaborate on given branches (e.g. fairwaves,
sysmocom, ...), we can add specific namespaces for these groups. I have so far
added a refs/heads/sysmocom/* namespace and a "sysmocom group access" gerrit
group. I can add more groups on request. The only advantage here is that you
can drop the "users/" path element. Any "known users" member can collaborate on
users/* branches already, e.g. refs/heads/users/sysmocom/topic.
To make this less config prone, we could go one step further and allow global
push access to the group of known users, going back to the model of trust that
users take care not to push nonsense. That would include access to master. We
also still have various branches in git.osmocom.org that don't have the 'users'
path element. If we grant global namespace access to known users, these would
continue to be useful. If not, we could rename these to users/ to allow further
access.
Opinions welcome!
~Neels
--
- Neels Hofmeyr <nhofmeyr(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschäftsführer / Managing Directors: Holger Freyther, Harald Welte
Review at https://gerrit.osmocom.org/182
dyn pdch: send PDCH ACT for each TCH/F_PDCH on TRX RSL UP
Add dyn_pdch_init() and call from inp_sig_cb() upon RSL UP.
Revert the |= TS_F_PDCH_MODE chunk from previous commit, since this flag will
now be set after dyn_pdch_init() sent out the PDCH ACT, i.e. when the PDCH ACT
ACK messages are received.
Change-Id: I7bfc70527162c95b3d7ea853eda6376b4f1f1161
---
M openbsc/src/libbsc/bsc_init.c
1 file changed, 34 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/82/182/1
diff --git a/openbsc/src/libbsc/bsc_init.c b/openbsc/src/libbsc/bsc_init.c
index 110e160..02c949c 100644
--- a/openbsc/src/libbsc/bsc_init.c
+++ b/openbsc/src/libbsc/bsc_init.c
@@ -302,6 +302,36 @@
generate_ma_for_ts(&trx->ts[i]);
}
+static void dyn_pdch_init(struct gsm_bts_trx *trx)
+{
+ unsigned int i;
+ struct gsm_bts_trx_ts *ts;
+ int rc;
+ unsigned int pdch_act_count = 0;
+
+ for (i = 0; i < TRX_NR_TS; i++) {
+ ts = &trx->ts[i];
+ if (ts->pchan == GSM_PCHAN_TCH_F_PDCH) {
+ rc = rsl_ipacc_pdch_activate(ts, 1);
+ if (rc != 0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "Failed to activate PDCH on"
+ " BTS %u TRX %u TS %u: %d\n",
+ trx->bts->nr, trx->nr, i, rc);
+ continue;
+ }
+ pdch_act_count ++;
+ }
+ }
+
+ if (pdch_act_count) {
+ LOGP(DRSL, LOGL_NOTICE,
+ "Activated PDCH on %u dynamic TCH/F_PDCH time slots"
+ " for BTS %u TRX %u\n",
+ pdch_act_count, trx->bts->nr, trx->nr);
+ }
+}
+
/* Callback function to be called every time we receive a signal from INPUT */
static int inp_sig_cb(unsigned int subsys, unsigned int signal,
void *handler_data, void *signal_data)
@@ -328,14 +358,14 @@
generate_cell_chan_list(ca, trx->bts);
llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) {
- for (i = 0; i < ARRAY_SIZE(cur_trx->ts); i++) {
+ for (i = 0; i < ARRAY_SIZE(cur_trx->ts); i++)
generate_ma_for_ts(&cur_trx->ts[i]);
- cur_trx->ts[i].flags |= TS_F_PDCH_MODE;
- }
}
}
- if (isd->link_type == E1INP_SIGN_RSL)
+ if (isd->link_type == E1INP_SIGN_RSL) {
bootstrap_rsl(trx);
+ dyn_pdch_init(trx);
+ }
break;
case S_L_INP_TEI_DN:
LOGP(DLMI, LOGL_ERROR, "Lost some E1 TEI link: %d %p\n", isd->link_type, trx);
--
To view, visit https://gerrit.osmocom.org/182
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bfc70527162c95b3d7ea853eda6376b4f1f1161
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr(a)sysmocom.de>