5 Jun
2009
5 Jun
'09
2:15 a.m.
Hey,
in patch22 Andreas pointed out that we might have unbalanced ref count in the
location updating request path(s) and these patches try to fix some possible
reliable issues, handle spoofing to a certain degree and recover from error
(abnormal channel close).
include/openbsc/signal.h | 10 ++++++++++
src/chan_alloc.c | 4 +++-
src/gsm_04_08.c | 32 ++++++++++++++++++++++++--------
3 files changed, 37 insertions(+), 9 deletions(-)
5 Jun
5 Jun
2:16 a.m.
New subject: [PATCH 1/3] Remove use_lchan and put_lchan from location updating
Andreas Eversberg is suspecting that some of these messages
are not balanced and we are seeing a leak here. The general location
updating request is guarded by the "location updating request" object
inside the lchan that will keep the channel open for at least five
seconds to get all the information we need.
---
src/gsm_04_08.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/src/gsm_04_08.c b/src/gsm_04_08.c
index fd47d8b..a91ef7a 100644
--- a/src/gsm_04_08.c
+++ b/src/gsm_04_08.c
@@ -481,11 +481,6 @@ static int mm_rx_id_resp(struct msgb *msg)
DEBUGP(DMM, "IDENTITY RESPONSE: mi_type=0x%02x MI(%s)\n",
mi_type, mi_string);
- /*
- * Rogue messages could trick us but so is life
- */
- put_lchan(lchan);
-
switch (mi_type) {
case GSM_MI_TYPE_IMSI:
if (!lchan->subscr)
@@ -573,7 +568,6 @@ static int mm_rx_loc_upd_req(struct msgb *msg)
switch (mi_type) {
case GSM_MI_TYPE_IMSI:
/* we always want the IMEI, too */
- use_lchan(lchan);
rc = mm_tx_identity_req(lchan, GSM_MI_TYPE_IMEI);
lchan->loc_operation->waiting_for_imei = 1;
@@ -582,7 +576,6 @@ static int mm_rx_loc_upd_req(struct msgb *msg)
break;
case GSM_MI_TYPE_TMSI:
/* we always want the IMEI, too */
- use_lchan(lchan);
rc = mm_tx_identity_req(lchan, GSM_MI_TYPE_IMEI);
lchan->loc_operation->waiting_for_imei = 1;
@@ -590,7 +583,6 @@ static int mm_rx_loc_upd_req(struct msgb *msg)
subscr = subscr_get_by_tmsi(mi_string);
if (!subscr) {
/* send IDENTITY REQUEST message to get IMSI */
- use_lchan(lchan);
rc = mm_tx_identity_req(lchan, GSM_MI_TYPE_IMSI);
lchan->loc_operation->waiting_for_imsi = 1;
}
--
1.6.3.1
2:17 a.m.
New subject: [PATCH 2/3] gsm_04_08.c: Some spoofing protection against two location
Do not allow two location updating requests at the same time on the
same lchan. Such an event is certainly spoofed and can confuse the
internal logic of the application. Prevent that.
---
src/gsm_04_08.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/gsm_04_08.c b/src/gsm_04_08.c
index a91ef7a..18371b5 100644
--- a/src/gsm_04_08.c
+++ b/src/gsm_04_08.c
@@ -563,6 +563,12 @@ static int mm_rx_loc_upd_req(struct msgb *msg)
DEBUGP(DMM, "LUPDREQ: mi_type=0x%02x MI(%s) type=%s\n", mi_type, mi_string,
lupd_name(lu->type));
+ /* two location updating request on the same lchan... *spoof* */
+ if (lchan->loc_operation) {
+ gsm0408_loc_upd_rej(lchan, GSM48_REJECT_PROTOCOL_ERROR);
+ return 0;
+ }
+
allocate_loc_updating_req(lchan);
switch (mi_type) {
--
1.6.3.1
2:18 a.m.
New subject: [PATCH 3/3] lchan: Handle the abnormal case of channel getting closed
The abnormal case is that lchan_free ist getting called due
a RSL_MT_CHAN_REL_ACK in the RSL but the refcount of this
channel is not zero. This means that some "logical operation"
is still going on that needs to be cancelled.
Instead of always queuing up all operations in the
struct lchan use the signal framework to inform higher layers
about this abnormal case.
In gsm_04_08.c a signal handler is installed and in the
abnormal case the location updating request operation is
freed.
---
include/openbsc/signal.h | 10 ++++++++++
src/chan_alloc.c | 4 +++-
src/gsm_04_08.c | 18 ++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/include/openbsc/signal.h b/include/openbsc/signal.h
index c2cf46a..ca16296 100644
--- a/include/openbsc/signal.h
+++ b/include/openbsc/signal.h
@@ -37,6 +37,7 @@ enum signal_subsystems {
SS_SMS,
SS_ABISIP,
SS_NM,
+ SS_LCHAN,
};
/* SS_PAGING signals */
@@ -55,6 +56,15 @@ enum signal_nm {
S_NM_FAIL_REP, /* GSM 12.21 failure event report */
};
+/* SS_LCHAN signals */
+enum signal_lchan {
+ /*
+ * The lchan got freed with refcount != 0 and error
+ * recovery needs to happen right now.
+ */
+ S_LCHAN_UNEXPECTED_RELEASE,
+};
+
typedef int signal_cbfn(unsigned int subsys, unsigned int signal,
void *handler_data, void *signal_data);
diff --git a/src/chan_alloc.c b/src/chan_alloc.c
index fa07273..77a4f57 100644
--- a/src/chan_alloc.c
+++ b/src/chan_alloc.c
@@ -31,6 +31,7 @@
#include <openbsc/abis_nm.h>
#include <openbsc/abis_rsl.h>
#include <openbsc/debug.h>
+#include <openbsc/signal.h>
static void auto_release_channel(void *_lchan);
@@ -193,8 +194,9 @@ void lchan_free(struct gsm_lchan *lchan)
lchan->subscr = 0;
}
- /* We might kill an active channel... FIXME: call cancellations */
+ /* We might kill an active channel... */
if (lchan->use_count != 0) {
+ dispatch_signal(SS_LCHAN, S_LCHAN_UNEXPECTED_RELEASE, lchan);
lchan->use_count = 0;
}
diff --git a/src/gsm_04_08.c b/src/gsm_04_08.c
index 18371b5..3dfa780 100644
--- a/src/gsm_04_08.c
+++ b/src/gsm_04_08.c
@@ -182,6 +182,24 @@ static void allocate_loc_updating_req(struct gsm_lchan
*lchan)
memset(lchan->loc_operation, 0, sizeof(*lchan->loc_operation));
}
+static int gsm0408_handle_lchan_signal(unsigned int subsys, unsigned int
signal,
+ void *handler_data, void *signal_data)
+{
+ if (subsys != SS_LCHAN || signal != S_LCHAN_UNEXPECTED_RELEASE)
+ return 0;
+
+ /* give up on the loc_operation in case of error */
+ struct gsm_lchan *lchan = (struct gsm_lchan *)handler_data;
+ release_loc_updating_req(lchan);
+
+ return 0;
+}
+
+static __attribute__((constructor)) void on_load_0408(void)
+{
+ register_signal_handler(SS_LCHAN, gsm0408_handle_lchan_signal, NULL);
+}
+
static void to_bcd(u_int8_t *bcd, u_int16_t val)
{
bcd[2] = val % 10;
--
1.6.3.1
5500
days inactive
5500
days old
3 comments
1 participants
participants (1)
-
Holger Freyther