Change in osmo-msc[master]: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
13 Jan
2022
13 Jan
'22
3:48 p.m.
pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-msc/+/26917 )
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
call_leg: local_bridge: Avoid null pointer access if CN-side not ready
This happens if for instance an HNBGW drops the RAB-AssignmentRequest
and does nothing with it.
call_leg.c:348:15: runtime error: member access within null pointer of type 'struct
rtp_stream'
Related: OS#5401
Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
---
M src/libmsc/call_leg.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/17/26917/1
diff --git a/src/libmsc/call_leg.c b/src/libmsc/call_leg.c
index 6242a80..e890f75 100644
--- a/src/libmsc/call_leg.c
+++ b/src/libmsc/call_leg.c
@@ -344,6 +344,9 @@
}
codec = cl1->rtp[RTP_TO_RAN]->codec;
+ if (!cl1->rtp[RTP_TO_CN] || !cl2->rtp[RTP_TO_CN])
+ return -ENOTCONN;
+
call_leg_ensure_ci(cl1, RTP_TO_CN, call_id1, trans1,
&codec, &cl2->rtp[RTP_TO_CN]->local);
call_leg_ensure_ci(cl2, RTP_TO_CN, call_id2, trans2,
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
13 Jan
13 Jan
3:54 p.m.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1:
This patch promptly terminates the call if any of the 2 CN-sides are not already ready
when the bridging is requested.
Another version of the patch which also would fix the issue would be:
"""
- call_leg_ensure_ci(cl1, RTP_TO_CN, call_id1, trans1,
- &codec, &cl2->rtp[RTP_TO_CN]->local);
- call_leg_ensure_ci(cl2, RTP_TO_CN, call_id2, trans2,
- &codec, &cl1->rtp[RTP_TO_CN]->local);
+ call_leg_ensure_ci(cl1, RTP_TO_CN, call_id1, trans1, &codec,
+ cl2->rtp[RTP_TO_CN] ? &cl2->rtp[RTP_TO_CN]->local
: NULL);
+ call_leg_ensure_ci(cl2, RTP_TO_CN, call_id2, trans2, &codec,
+ cl1->rtp[RTP_TO_CN] ? &cl1->rtp[RTP_TO_CN]->local
: NULL)
"""
In the version presented above, the call would continue for a few more secs, until timer
X2 triggers in rtp_stream FSM.
The question here is whether in normal conditions, it can be that the RAB-ASsReq is
delayed so much that it arrives after the bridging is done. In that case, maybe this
version above is better since then it may end up working correctly...
Neels or others, any comments on preferred way to fix it?
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 13 Jan 2022 14:54:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
6:49 p.m.
laforge has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/26917 )
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1:
The question here is whether in normal conditions, it
can be that the RAB-ASsReq is delayed so much that it arrives after the bridging is done.
In that case, maybe this version above is better since then it may end up working
correctly...
well, the RAB assignment might take quite a long time e.g. in a satellite back-haul
scenario, where the hNB is 300ms RTT away.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 13 Jan 2022 17:49:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
14 Jan
14 Jan
7:25 p.m.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1: Code-Review+1
I'm wondering why call_leg_local_bridge() is even called before all sides are ready.
I think placing a safeguard here to avoid a crash is a good thing (fix DoS).
But if the RAB Assignment didn't complete, it should not even be reaching this stage,
right?
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 14 Jan 2022 18:25:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
7:47 p.m.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1:
Patch Set 1: Code-Review+1
I'm wondering why call_leg_local_bridge() is even called before all sides are ready.
I think placing a safeguard here to avoid a crash is a good thing (fix DoS).
But if the RAB Assignment didn't complete, it should not even be reaching this stage,
right?
Agree, I also find this a bit weird that it reaches this point. I guess on the 3g side (MO
side) the call proceeds due to CM Service Request, and is not currently waiting for RAB
Assignment-Response?
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 14 Jan 2022 18:47:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
17 Jan
17 Jan
2:46 p.m.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1: Verified+1 Code-Review+1
I have observed the problem also. This patch prevents the crash.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 17 Jan 2022 13:46:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
3:01 p.m.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1:
Patch Set 1:
I think the point is that the RAB-AssignmentRequest does complete, but the
RAB-AssignmentResponse wasn't forwarded by the hnbgw for some reason. The call then
proceeds. (Connect, Connect Acknowledge...) Thats at least how I can trigger the problem.
I just uncomment the line where the modified RAB-AssignmentResponse is forwarded.
After all the HNB sends the RAB-AssignmentResponse and then the Connect. Due to the MGW
handling it is now possible that the Connect overtakes the RAB-AssignmentResponse, if the
operations on the MGW take very long. This is probably not a real world problem but maybe
we have to synchronize the Connect message. I am not sure. At least from the trace I can
see there are about 5 sec. between RAB-AssignmentResponse and Connect.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 17 Jan 2022 14:01:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Patch Set 1: Code-Review+1
I'm wondering why call_leg_local_bridge() is even called before all sides are ready.
I think placing a safeguard here to avoid a crash is a good thing (fix DoS).
But if the RAB Assignment didn't complete, it should not even be reaching this stage,
right?
Agree, I also find this a bit weird that it reaches this point. I guess on the 3g side
(MO side) the call proceeds due to CM Service Request, and is not currently waiting for
RAB Assignment-Response?
3:12 p.m.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917
)
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 17 Jan 2022 14:12:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
3:12 p.m.
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/26917 )
Change subject: call_leg: local_bridge: Avoid null pointer access if CN-side not ready
......................................................................
call_leg: local_bridge: Avoid null pointer access if CN-side not ready
This happens if for instance an HNBGW drops the RAB-AssignmentRequest
and does nothing with it.
call_leg.c:348:15: runtime error: member access within null pointer of type 'struct
rtp_stream'
Related: OS#5401
Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
---
M src/libmsc/call_leg.c
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
dexter: Looks good to me, but someone else must approve; Verified
neels: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/libmsc/call_leg.c b/src/libmsc/call_leg.c
index 6242a80..e890f75 100644
--- a/src/libmsc/call_leg.c
+++ b/src/libmsc/call_leg.c
@@ -344,6 +344,9 @@
}
codec = cl1->rtp[RTP_TO_RAN]->codec;
+ if (!cl1->rtp[RTP_TO_CN] || !cl2->rtp[RTP_TO_CN])
+ return -ENOTCONN;
+
call_leg_ensure_ci(cl1, RTP_TO_CN, call_id1, trans1,
&codec, &cl2->rtp[RTP_TO_CN]->local);
call_leg_ensure_ci(cl2, RTP_TO_CN, call_id2, trans2,
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/26917
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I67d2d5b2dd3b367c34f929d63c056306ec001431
Gerrit-Change-Number: 26917
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: merged
853
days inactive
857
days old
8 comments
4 participants
participants (4)
-
dexter -
laforge
-
neels
-
pespin