fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31991 ) Change subject: osmo-bts-{sysmo,lc15,oc2g}: fix segfault in ph_tch_req() ...................................................................... osmo-bts-{sysmo,lc15,oc2g}: fix segfault in ph_tch_req() ph_tch_req() is a recursive function and conditionally calls itself at the very bottom. The recursive call happens iff all of the following conditions are met: * DTXd is enabled, * AMR codec is in use, * DTX DL AMR FSM state is recursive. The problem is that ph_tch_req() may pull sizeof(*lsap) from the given msgb twice: during the initial and the recursive calls. The second attempt to pull sizeof(*lsap) causes the process to abort, because the remaining room is less than it's attempting to pull. AFAICT, doing msgb_pull() is not really necessary, given that l1sap_tch_rts_ind() thankfully does set msg->l2h before pushing the lsap header in front of the actual frame. Update osmo-bts-sysmo and its copy-pasted siblings, which are likely affected too, except osmo-bts-octphy which does not do the recursion. Change-Id: Ib349b74a9e4bd48c902286f872d3b0e9a068256c Related: OS#5925 --- M src/osmo-bts-lc15/l1_if.c M src/osmo-bts-oc2g/l1_if.c M src/osmo-bts-octphy/l1_if.c M src/osmo-bts-sysmo/l1_if.c 4 files changed, 34 insertions(+), 8 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve keith: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/osmo-bts-lc15/l1_if.c b/src/osmo-bts-lc15/l1_if.c index ac165b8..987b6e3 100644 --- a/src/osmo-bts-lc15/l1_if.c +++ b/src/osmo-bts-lc15/l1_if.c @@ -508,7 +508,6 @@ /* create new message and fill data */ if (msg) { - msgb_pull(msg, sizeof(*l1sap)); /* create new message */ nmsg = l1p_msgb_alloc(); if (!nmsg) @@ -517,7 +516,7 @@ rc = l1if_tch_encode(lchan, l1p->u.phDataReq.msgUnitParam.u8Buffer, &l1p->u.phDataReq.msgUnitParam.u8Size, - msg->data, msg->len, u32Fn, use_cache, + msgb_l2(msg), msgb_l2len(msg), u32Fn, use_cache, l1sap->u.tch.marker); if (rc < 0) { /* no data encoded for L1: smth will be generated below */ diff --git a/src/osmo-bts-oc2g/l1_if.c b/src/osmo-bts-oc2g/l1_if.c index 194f82a..3308a46 100644 --- a/src/osmo-bts-oc2g/l1_if.c +++ b/src/osmo-bts-oc2g/l1_if.c @@ -561,7 +561,6 @@ /* create new message and fill data */ if (msg) { - msgb_pull(msg, sizeof(*l1sap)); /* create new message */ nmsg = l1p_msgb_alloc(); if (!nmsg) @@ -570,7 +569,7 @@ rc = l1if_tch_encode(lchan, l1p->u.phDataReq.msgUnitParam.u8Buffer, &l1p->u.phDataReq.msgUnitParam.u8Size, - msg->data, msg->len, u32Fn, use_cache, + msgb_l2(msg), msgb_l2len(msg), u32Fn, use_cache, l1sap->u.tch.marker); if (rc < 0) { /* no data encoded for L1: smth will be generated below */ diff --git a/src/osmo-bts-octphy/l1_if.c b/src/osmo-bts-octphy/l1_if.c index 110f8a3..4898eb6 100644 --- a/src/osmo-bts-octphy/l1_if.c +++ b/src/osmo-bts-octphy/l1_if.c @@ -593,7 +593,6 @@ return -ENOMEM; } - msgb_pull(msg, sizeof(*l1sap)); tOCTVC1_GSM_MSG_TRX_REQUEST_LOGICAL_CHANNEL_DATA_CMD *data_req = (tOCTVC1_GSM_MSG_TRX_REQUEST_LOGICAL_CHANNEL_DATA_CMD *) msgb_put(nmsg, sizeof(*data_req)); @@ -615,7 +614,7 @@ &data_req->Data.ulPayloadType, data_req->Data.abyDataContent, &data_req->Data.ulDataLength, - msg->data, msg->len); + msgb_l2(msg), msgb_l2len(msg)); mOCTVC1_GSM_MSG_TRX_REQUEST_LOGICAL_CHANNEL_DATA_CMD_SWAP(data_req); } else { diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c index 5c99824..646cf01 100644 --- a/src/osmo-bts-sysmo/l1_if.c +++ b/src/osmo-bts-sysmo/l1_if.c @@ -505,7 +505,6 @@ /* create new message and fill data */ if (msg) { - msgb_pull(msg, sizeof(*l1sap)); /* create new message */ nmsg = l1p_msgb_alloc(); if (!nmsg) @@ -514,7 +513,7 @@ rc = l1if_tch_encode(lchan, l1p->u.phDataReq.msgUnitParam.u8Buffer, &l1p->u.phDataReq.msgUnitParam.u8Size, - msg->data, msg->len, u32Fn, use_cache, + msgb_l2(msg), msgb_l2len(msg), u32Fn, use_cache, l1sap->u.tch.marker); if (rc < 0) { /* no data encoded for L1: smth will be generated below */ -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31991 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ib349b74a9e4bd48c902286f872d3b0e9a068256c Gerrit-Change-Number: 31991 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: keith <keith@rhizomatica.org> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-MessageType: merged