From 6463548e6f689b34551821d135b3380323b76504 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 9 Aug 2026 17:39:14 +0200 Subject: [PATCH 01/15] Fix Nmb2 PATCH JSON-Pointer path for MBSTF DistSession update TS 29.581 defines PATCH /dist-sessions/{distSessionRef} as operating on the flat DistSession resource directly, with no wrapper property. The JSON-Patch built here addressed paths under a non-existent /distSession prefix instead: '' for a full replace (RFC 6901: the whole document is the empty JSON Pointer) and /distSessionState for the state-only case. A conformant MBSTF peer would reject these patches; this only worked because the paired MBSTF side independently tolerates the same non-standard prefix. --- src/mbsf/MBSMFMBSSession.cc | 16 +++++++++++++++- src/mbsf/Nmb2Build.cc | 9 +++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/mbsf/MBSMFMBSSession.cc b/src/mbsf/MBSMFMBSSession.cc index c6bec1e..0f22d86 100644 --- a/src/mbsf/MBSMFMBSSession.cc +++ b/src/mbsf/MBSMFMBSSession.cc @@ -248,6 +248,16 @@ bool MBSMFMBSSession::processEvent(Open5GSEvent &MBSMFEvent) } UserDataIngSession::setMBSSessionFlag(*ids); } else if (mbsf_event->result == OGS_ERROR) { + // BUG FIX: the fallback call below used to run unconditionally, so even + // when a specific cause was already matched and handled just above (e.g. + // the registered 403 MBS_DIST_SESSION_ALREADY_CREATED), it was immediately + // overwritten by a second, generic INBOUND_SERVER_ERROR call with no + // problem_detail -- meaning the client only ever saw the generic 502-class + // error regardless of what MB-SMF actually reported. Track whether a cause + // (specific or the "no cause string" generic-with-detail case) was already + // handled and only fall through to the bare generic call as a genuine + // last resort (no problem_details at all, or an unregistered cause string). + bool cause_handled = false; if (mbsf_event->problem_details) { cJSON *problem = OpenAPI_problem_details_convertToJSON((OpenAPI_problem_details_t*)mbsf_event->problem_details); CJson problem_detail(problem, true); @@ -256,12 +266,16 @@ bool MBSMFMBSSession::processEvent(Open5GSEvent &MBSMFEvent) MBSProblemCause::lookup(std::string(mbsf_event->problem_details->cause)); if (cause.has_value()) { UserDataIngSession::setMBSSessionFailureFlag(*ids, cause.value(), problem_detail); + cause_handled = true; } } else { UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR, problem_detail); + cause_handled = true; } } - UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR); + if (!cause_handled) { + UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR); + } } else { UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR); } diff --git a/src/mbsf/Nmb2Build.cc b/src/mbsf/Nmb2Build.cc index ecc4a10..9f2c473 100644 --- a/src/mbsf/Nmb2Build.cc +++ b/src/mbsf/Nmb2Build.cc @@ -242,7 +242,10 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat std::shared_ptr context_data_ptr(ing_session->getDistributionSessionInfoData(session_ids->second->second)); DistSessionState req_state; if (context_data_ptr->needsUpdate) { - status_item.path = (char *)"/distSession"; + // TS 29.581: PATCH /dist-sessions/{distSessionRef} operates on the flat + // DistSession resource directly, with no "distSession" wrapper property. + // RFC 6901: the whole document is addressed by the empty JSON Pointer "". + status_item.path = (char *)""; std::shared_ptr dist_session = build_nmb2_create_dist_session(ing_session, context_data_ptr); std::string sess_id(context_data_ptr->mbstfDistSessionId); @@ -264,7 +267,9 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat req_state = want_state; } patch_val = req_state.toJSON(); - status_item.path = (char *)"/distSession/distSessionState"; + // Flat DistSession resource: the field is at "/distSessionState", + // not under a non-existent "/distSession" wrapper (see above). + status_item.path = (char *)"/distSessionState"; } } From ff5b2150767c2b18d05741fcf14081f530ee6beb Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 9 Aug 2026 17:39:41 +0200 Subject: [PATCH 02/15] Reject PATCH/PUT changing objDistrInfo/pckDistrInfo while not INACTIVE These fields were previously only ever copied across when the Distribution Session is INACTIVE -- if PATCHed while ESTABLISHED/ACTIVE the request was accepted (200) but the change was silently dropped, so e.g. a PATCH narrowing objAcqIds would echo back the old array. Reject the request outright (ModelException, MODIFICATION_NOT_ALLOWED) instead of silently no-op'ing it. --- src/mbsf/DistributionSessionInfo.cc | 33 ++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/mbsf/DistributionSessionInfo.cc b/src/mbsf/DistributionSessionInfo.cc index 0001753..0c009fb 100644 --- a/src/mbsf/DistributionSessionInfo.cc +++ b/src/mbsf/DistributionSessionInfo.cc @@ -162,6 +162,21 @@ CJson DistributionSessionInfo::json(bool as_request = false) const return m_mbsDistributionSessionInfo->toJSON(as_request); } +namespace { +// Compares two optional> fields, treating "absent" and "present but null" as the +// same "no value" state so a field that's merely re-sent unchanged doesn't spuriously trip up as +// "changed". +template +bool optionalPtrFieldsEqual(const std::optional> &a, const std::optional> &b) +{ + bool a_has = a.has_value() && a.value(); + bool b_has = b.has_value() && b.value(); + if (a_has != b_has) return false; + if (!a_has) return true; + return *a.value() == *b.value(); +} +} + std::shared_ptr &DistributionSessionInfo::updateMBSDistributionSessionInfo( std::shared_ptr new_mbs_dist_session_infos) { @@ -179,8 +194,24 @@ std::shared_ptr &DistributionSessionInfo::updateMBSD // 2. Conditional updates – only when the session is INACTIVE // -------------------------------------------------------------------- std::optional > dist_session_state = m_mbsDistributionSessionInfo->getMbsDistSessState(); + bool is_inactive = dist_session_state.has_value() && dist_session_state.value()->getValue() == DistSessionState::VAL_INACTIVE; + + // BUG FIX: objDistrInfo/pckDistrInfo (and hence e.g. objAcqIds) were previously only ever + // copied across below when the Distribution Session is INACTIVE -- if PATCHed while + // ESTABLISHED/ACTIVE the request was accepted (200) but the change was silently dropped, so + // e.g. a PATCH narrowing objAcqIds would echo back the OLD array. These fields are only + // mutable while INACTIVE, so reject the request outright instead of silently no-op'ing it. + if (!is_inactive) { + if (!optionalPtrFieldsEqual(m_mbsDistributionSessionInfo->getObjDistrInfo(), new_mbs_dist_session_infos->getObjDistrInfo()) || + !optionalPtrFieldsEqual(m_mbsDistributionSessionInfo->getPckDistrInfo(), new_mbs_dist_session_infos->getPckDistrInfo())) { + throw ModelException( + "objDistrInfo/pckDistrInfo cannot be modified while the MBS Distribution Session is not INACTIVE", + "MBSDistributionSessionInfo", "objDistrInfo", + fiveg_mag_reftools::ProblemCause::MODIFICATION_NOT_ALLOWED); + } + } - if (dist_session_state.has_value() && dist_session_state.value()->getValue() == DistSessionState::VAL_INACTIVE) { + if (is_inactive) { // ----- Max Continuous Bit Rate ----- m_mbsDistributionSessionInfo->setMaxContBitRate(std::move(new_mbs_dist_session_infos->getMaxContBitRate())); From 488d4574ad007029b6c96839acd2dcd6403f1e3f Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 9 Aug 2026 17:40:22 +0200 Subject: [PATCH 03/15] Stop gating ExtTgtServAreas/NrRedCapUeInfo updates on INACTIVE state TS 26.502 clause 4.5.6 lists external target service areas and the NR RedCap UE class among the parameters the MBS Application Provider may update at any time, alongside mbsServInfo/mbsFSAId/tgtServAreas -- not restricted to INACTIVE like the fields below them. A PATCH/PUT changing either while the session was ACTIVE/ESTABLISHED was previously silently dropped instead of applied. --- src/mbsf/DistributionSessionInfo.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mbsf/DistributionSessionInfo.cc b/src/mbsf/DistributionSessionInfo.cc index 0c009fb..74bd33f 100644 --- a/src/mbsf/DistributionSessionInfo.cc +++ b/src/mbsf/DistributionSessionInfo.cc @@ -190,6 +190,12 @@ std::shared_ptr &DistributionSessionInfo::updateMBSD m_mbsDistributionSessionInfo->setTgtServAreas(std::move(new_mbs_dist_session_infos->getTgtServAreas())); + // TS 26.502 clause 4.5.6 lists these among the parameters the MBS Application + // Provider may update at any time, alongside mbsServInfo/mbsFSAId/tgtServAreas + // above -- not gated on INACTIVE like the block below. + m_mbsDistributionSessionInfo->setExtTgtServAreas(std::move(new_mbs_dist_session_infos->getExtTgtServAreas())); + m_mbsDistributionSessionInfo->setNrRedCapUeInfo(std::move(new_mbs_dist_session_infos->getNrRedCapUeInfo())); + // -------------------------------------------------------------------- // 2. Conditional updates – only when the session is INACTIVE // -------------------------------------------------------------------- @@ -233,18 +239,12 @@ std::shared_ptr &DistributionSessionInfo::updateMBSD // ----- Traffic Marking Info ----- m_mbsDistributionSessionInfo->setTrafficMarkingInfo(std::move(new_mbs_dist_session_infos->getTrafficMarkingInfo())); - // ----- External Target Service Areas ----- - m_mbsDistributionSessionInfo->setExtTgtServAreas(std::move(new_mbs_dist_session_infos->getExtTgtServAreas())); - // ----- Multiplexed Service Flag ----- m_mbsDistributionSessionInfo->setMultiplexedServFlag(std::move(new_mbs_dist_session_infos->getMultiplexedServFlag())); // ----- Restricted Flag ----- m_mbsDistributionSessionInfo->setRestrictedFlag(std::move(new_mbs_dist_session_infos->getRestrictedFlag())); - // ----- NR RedCap UE Info ----- - m_mbsDistributionSessionInfo->setNrRedCapUeInfo(std::move(new_mbs_dist_session_infos->getNrRedCapUeInfo())); - // ----- Associated Session Id ----- m_mbsDistributionSessionInfo->setAssociatedSessionId(std::move(new_mbs_dist_session_infos->getAssociatedSessionId())); } From 20c713b13fbd4056e8287a7d3055608da4241828 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 9 Aug 2026 17:40:22 +0200 Subject: [PATCH 04/15] Preserve MNC digit count when building NCGI/TAI PLMN Ids mcc()/mnc() converted the spec's digit strings to plain uint16_t, discarding whether the MNC has 2 or 3 digits (and any leading zero). Downstream code then re-guessed the digit count from the numeric value (mnc<100?2:3), misclassifying any real 3-digit MNC under 100 (e.g. "001"-"099") as 2-digit. Added mncLen() (from the source string's actual length) and wired both call sites to the new length-aware mb_smf_sc_ncgi_set_plmn_id_len()/mb_smf_sc_tai_new_len() functions in rt-5gc-service-consumers instead of the guessing ones. --- src/mbsf/MBSNcgi.cc | 4 +++- src/mbsf/MBSPlmnId.hh | 4 ++++ src/mbsf/TrackingAreaIdentity.cc | 13 ++++++------- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/mbsf/MBSNcgi.cc b/src/mbsf/MBSNcgi.cc index fdd5cda..3546df0 100644 --- a/src/mbsf/MBSNcgi.cc +++ b/src/mbsf/MBSNcgi.cc @@ -81,7 +81,9 @@ mb_smf_sc_ncgi_t *MBSNcgi::populateNcgi() { mb_smf_sc_ncgi_t *ncgi = mb_smf_sc_ncgi_new(); - mb_smf_sc_ncgi_set_plmn_id(ncgi, mcc, mnc); + // Use the length-aware setter: mcc()/mnc() alone lose the MNC's actual digit + // count (2 vs 3), which a plain numeric value under 100 cannot distinguish. + mb_smf_sc_ncgi_set_plmn_id_len(ncgi, mcc, mnc, mbs_plmn_id->mncLen()); uint64_t cell_id = nrCellId(); ncgi->nr_cell_id = static_cast(cell_id) & ((1ULL << 36) - 1); ncgi->nid = nid(); diff --git a/src/mbsf/MBSPlmnId.hh b/src/mbsf/MBSPlmnId.hh index 6f6c78e..b1aa734 100644 --- a/src/mbsf/MBSPlmnId.hh +++ b/src/mbsf/MBSPlmnId.hh @@ -56,6 +56,10 @@ public: uint16_t mcc(); uint16_t mnc(); + // The MNC's actual digit count (2 or 3), taken directly from the source + // string rather than guessed from mnc()'s numeric value -- a 3-digit MNC + // under 100 (e.g. "001") is otherwise indistinguishable from a 2-digit one. + uint8_t mncLen() const {return static_cast(getMnc().length());}; private: std::shared_ptr m_plmnId; diff --git a/src/mbsf/TrackingAreaIdentity.cc b/src/mbsf/TrackingAreaIdentity.cc index eabd369..1ed00df 100644 --- a/src/mbsf/TrackingAreaIdentity.cc +++ b/src/mbsf/TrackingAreaIdentity.cc @@ -86,7 +86,9 @@ mb_smf_sc_tai_t *TrackingAreaIdentity::populateTai() { tracking_area = tac(); n_id = nid(); - return mb_smf_sc_tai_new(mcc, mnc, tracking_area, n_id); + // Use the length-aware constructor: mcc()/mnc() alone lose the MNC's actual + // digit count (2 vs 3), which a plain numeric value under 100 cannot distinguish. + return mb_smf_sc_tai_new_len(mcc, mnc, mbs_plmn_id->mncLen(), tracking_area, n_id); } uint32_t TrackingAreaIdentity::tac() { @@ -124,12 +126,9 @@ uint32_t TrackingAreaIdentity::tac() { uint64_t* TrackingAreaIdentity::nid() { const std::optional &nid = getNid(); if (!nid.has_value()) return nullptr; - uint64_t value = 0; - for (char ch : nid.value()) { - if (std::isdigit(static_cast(ch))) { - value = value * 10 + (ch - '0'); - } - } + // TS 29.571 Nid is an 11-character hex string (44-bit SNPN Network Id) -- + // parse as base 16, matching the correct sibling implementation MBSNcgi::nid(). + uint64_t value = std::stoull(nid.value(), nullptr, 16); uint64_t *result = static_cast(std::malloc(sizeof(uint64_t))); if (result != nullptr) { From 3c3f52e3b44af0fcaf16a453258f7bce4765c7fb Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 9 Aug 2026 17:40:22 +0200 Subject: [PATCH 05/15] Harden MBS User Data Ingest Session PUT validation Two gaps in processUserDataIngSessionUpdate()'s PUT path (PATCH on this resource is intentionally not implemented yet, so unaffected): - actPeriods and actPeriodsRepRule are mutually exclusive per TS 29.580 clause 6, but a request setting both silently prioritised actPeriods instead of being rejected, unlike the equivalent POST-path check. - mbsSessionId and locationDependent must never be updated after initial provisioning per TS 29.580 clause 5.3.2.4.2, but only mbsDistSessionId was actually restored from the stored value before the change-detection comparison -- the other two passed through unprotected. --- src/mbsf/UserDataIngSession.cc | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index 1ef6a3f..a3761dd 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -495,6 +495,26 @@ bool UserDataIngSession::processEvent(Open5GSEvent &event) ogs_debug("Patch Request Parsed JSON: %s", txt.c_str()); } + // Reject actPeriods/actPeriodsRepRule given together, mirroring the + // mutual-exclusion check validate_state_setting_options() already + // enforces on POST (TS 29.580 clause 6: the two are mutually exclusive). + // This is a PUT-only fix: PATCH on this resource is intentionally not + // implemented yet (returns 404 above), so it is not affected. + try { + MBSUserDataIngSession update_model(user_data_ing_sess_update, true); + if (update_model.getActPeriods() && update_model.getActPeriodsRepRule()) { + std::map invalid_params; + invalid_params["actPeriods"] = "actPeriods cannot be present if actPeriodsRepRule is present"; + invalid_params["actPeriodsRepRule"] = "actPeriodsRepRule cannot be present if actPeriods is present"; + ogs_assert(true == NfServer::sendError(stream, ProblemCause::OPTIONAL_IE_INCORRECT, 3, message, + app_meta, api, std::nullopt, std::nullopt, std::nullopt, invalid_params)); + return true; + } + } catch (ModelException &ex) { + send_model_error(ex, stream, 3, message, app_meta, api, "Problem with UserDataIngSession update", "Validating UserDataIngSession update"); + return true; + } + try { std::shared_ptr user_data_ing_sess = find(user_data_ing_session_id); user_data_ing_sess->processUserDataIngSessionUpdate(stream_id, request_ctx, user_data_ing_sess_update); @@ -1227,8 +1247,13 @@ void UserDataIngSession::processUserDataIngSessionUpdate(ogs_pool_id_t stream_id // update std::shared_ptr update_info = sess_info_update.value(); - // Copy old MBS Dist Session Id + // TS 29.580 clause 5.3.2.4.2: mbsSessionId, mbsDistSessionId and + // locationDependent shall never be updated after provisioning -- + // restore all three from the stored value before comparing/applying, + // not just mbsDistSessionId. update_info->setMbsDistSessionId(info->getMbsDistSessionId()); + update_info->setMbsSessionId(info->getMbsSessionId()); + update_info->setLocationDependent(info->getLocationDependent()); if (*update_info != *info) { context_data->needsUpdate = true; From 2ecd7003bfa1795c99828ebb9626e290fa911940 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 10:57:00 +0200 Subject: [PATCH 06/15] Fix MBSTF rejecting distSessionState PATCH with 'Unknown path in JSON Patch' MBSTF's actual PATCH target for /dist-sessions/{id} is CreateReqData, whose generated applyPatch() only recognises paths under its own "/distSession" property -- it does not expose DistSession's fields at the top level. The previous "/distSessionState" (no wrapper) path was rejected outright, meaning every state-only PATCH (activate/deactivate) silently failed and triggered a rollback -- including the one that activates the built-in "USER SERVICE ANNOUNCEMENT CHANNEL" session, i.e. the real MBS-4-MC broadcast Service Announcement carousel never got updated with newly-provisioned services. --- src/mbsf/Nmb2Build.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/mbsf/Nmb2Build.cc b/src/mbsf/Nmb2Build.cc index 9f2c473..12987f0 100644 --- a/src/mbsf/Nmb2Build.cc +++ b/src/mbsf/Nmb2Build.cc @@ -267,9 +267,20 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat req_state = want_state; } patch_val = req_state.toJSON(); - // Flat DistSession resource: the field is at "/distSessionState", - // not under a non-existent "/distSession" wrapper (see above). - status_item.path = (char *)"/distSessionState"; + // BUG FIX (found live, 2026-08-10): MBSTF's actual PATCH target for + // /dist-sessions/{id} is CreateReqData (see DistributionSession::_apiSessionPatch(), + // which patches distributionSessionReqData(), a CreateReqData), and CreateReqData's + // generated applyPatch() (CreateReqData.cc) only recognises paths under its own + // "/distSession" property -- it does NOT expose DistSession's fields at the top + // level. The previous "/distSessionState" (no wrapper) path was rejected by MBSTF + // with "Runtime Error: Unknown path in JSON Patch", which meant every state-only + // PATCH (activate/deactivate) silently failed and triggered a rollback -- including + // the one that activates the built-in "USER SERVICE ANNOUNCEMENT CHANNEL" session, + // i.e. the real MBS-4-MC broadcast Service Announcement carousel never got updated + // with newly-provisioned services. Confirmed against CreateReqData.cc's path_prefix + // dispatch: it matches "/distSession" then delegates the remainder ("/distSessionState") + // to the nested DistSession object, which does recognise it (see DistSession.cc). + status_item.path = (char *)"/distSession/distSessionState"; } } From 7b5d0a431c0df9ebcef28cd74ab2f4e43362e2f9 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 10:57:00 +0200 Subject: [PATCH 07/15] Propagate the real MBS User Service type instead of hardcoding MULTICAST createMbsSession() always sent MBS_SERVICE_TYPE_MULTICAST to the SMF/MB-SMF regardless of the parent MBS User Service's own servType. SMF's Nmbsmf handler only triggers the Namf_MBSBroadcast context-create call -- the step that actually drives NGAP Broadcast Session Setup to the gNB -- if the service type is broadcast; for MULTICAST it correctly does nothing there (multicast UE-join uses a separate, currently-unimplemented Namf_MBSCommunication procedure instead). So every BROADCAST User Service ended up silently treated as MULTICAST at the MB-SMF boundary: PFCP/N4mb and MBSTF FLUTE transmission all completed normally, but NGAP never reached the gNB, no MRB was ever created for the new session, and content had no bearer to travel over -- dropped after leaving the UPF with no visible error anywhere. The actual servType has to be captured at construction time in the owning UserDataIngSession instance (which has access to mbsUserService()), not looked up later via a static locate(ingSessionId) call -- that lookup races against this object's own registration into the id->instance map and always loses (the ContextData is built and createMbsSession() invoked on it before the constructing UserDataIngSession finishes registering itself), silently falling back to MULTICAST regardless. --- src/mbsf/UserDataIngSession.cc | 27 ++++++++++++++++++++++++--- src/mbsf/UserDataIngSession.hh | 8 ++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index a3761dd..6949a84 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -1094,7 +1094,10 @@ void UserDataIngSession::updateContexts(ogs_pool_id_t stream_id, const std::shar .ssm_port = port, .request = request, .streamId = stream_id, - .tsi = tsi + .tsi = tsi, + // See createMbsSession()'s comment: captured here (an + // instance method, has "this") rather than looked up later. + .userServType = mbsUserService() ? mbsUserService()->getMBSUserServiceType() : std::string{} }); addToDistributionSessionInfos(key, ctx_data); createMbsSession(ctx_data); @@ -1171,7 +1174,11 @@ void UserDataIngSession::userServiceAnnChannelDistributionSessionInfo() .ssm_port = port, .request = nullptr, .streamId = 0, - .tsi = tsi + .tsi = tsi, + // This is the built-in Service Announcement carousel + // channel (see this method's name) -- MBS-4-MC Service + // Announcement is inherently a broadcast delivery, always. + .userServType = std::string("BROADCAST") }); addToDistributionSessionInfos(key, ctx_data); nmbstfDiscoverOnly(ctx_data); @@ -1732,7 +1739,21 @@ bool UserDataIngSession::createMbsSession(const std::shared_ptrsetTunnelRequest(true); mb_smf_mbs_session->setTmgiRequest(true); - mb_smf_mbs_session->setServiceType(MBS_SERVICE_TYPE_MULTICAST); + // BUG FIX (found live, 2026-08-10): this unconditionally sent MULTICAST to the SMF/MB-SMF + // regardless of the parent MBS User Service's own servType. SMF's Nmbsmf handler + // (n4mb-handler.c) only triggers the Namf_MBSBroadcast context-create call -- the step + // that actually drives NGAP Broadcast Session Setup to the gNB -- "if the service type is + // broadcast service" (TS 23.247 cl.7.3.1 step 2); for MULTICAST it correctly does nothing + // here (multicast UE-join uses a separate, currently-unimplemented Namf_MBSCommunication + // procedure instead). So every BROADCAST User Service ended up silently treated as + // MULTICAST at the MB-SMF boundary: PFCP/N4mb and MBSTF FLUTE transmission all completed + // normally, but NGAP never reached the gNB, no MRB was ever created for the new session, + // and content had no bearer to travel over -- dropped after leaving the UPF with no + // visible error anywhere. Confirmed via UserService::getMBSUserServiceType(), which reads + // the real value ("BROADCAST"/"MULTICAST") straight from the User Service's own servType. + mb_smf_mbs_session->setServiceType( + ogs_strcasecmp(context_data->userServType.c_str(), "BROADCAST") == 0 + ? MBS_SERVICE_TYPE_BROADCAST : MBS_SERVICE_TYPE_MULTICAST); if (!context_data->MBSSession) context_data->MBSSession = mb_smf_mbs_session; mb_smf_mbs_session->setCallback(UserDataIngDistSessId(context_data->ingSessionId, context_data->distSessionInfoKey)); populate_mb_smf_mbs_session(context_data, mb_smf_mbs_session); diff --git a/src/mbsf/UserDataIngSession.hh b/src/mbsf/UserDataIngSession.hh index 58b904a..c966c4f 100644 --- a/src/mbsf/UserDataIngSession.hh +++ b/src/mbsf/UserDataIngSession.hh @@ -124,6 +124,14 @@ public: reftools::mbsf::DistSessionState last_reported_state; std::shared_ptr distSession = nullptr; std::shared_ptr sdp = nullptr; + // BUG FIX (found live, 2026-08-10): captured here, at construction time in the owning + // UserDataIngSession instance method (which has "this" and so can call + // mbsUserService()), rather than looked up later inside the static createMbsSession() via + // locate(ingSessionId) -- that lookup raced against this object's own registration into + // the id->instance map and always lost (this ContextData is built and createMbsSession() + // is invoked on it *before* the constructing UserDataIngSession finishes registering + // itself), so it always silently fell back to "MULTICAST". See createMbsSession(). + std::string userServType = std::string{}; }; UserDataIngSession(fiveg_mag_reftools::CJson &json, bool as_request); From e75991d863a18841b1b9df0dbc80525b4c541b26 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 15:08:13 +0200 Subject: [PATCH 08/15] Fix announcement channel worker hanging forever on a lost MBSTF response sendMbstfRequests() used to be called exactly once (guarded by requested_mbstf_dist_session, which was never reset), and the wait following it had no deadline of its own. If MBSTF's response to that single request was ever lost, confirmed live via gdb: this worker thread stuck forever in the wait_for() below, with a real, established TCP connection to MBSTF sitting idle (some transient SBI/SCP hiccup around the same moment, not a deadlock or a bug in the wait itself) -- the announcement channel's distribution session would never be created and no Service Announcement content would ever be pushed, for the lifetime of the MBSF process, with no way to recover short of restarting it. Retry after a bounded number of wait iterations (5 seconds) instead of waiting on the same request forever. --- src/mbsf/UserServiceAnnChannel.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/mbsf/UserServiceAnnChannel.cc b/src/mbsf/UserServiceAnnChannel.cc index 68248f8..54687e4 100644 --- a/src/mbsf/UserServiceAnnChannel.cc +++ b/src/mbsf/UserServiceAnnChannel.cc @@ -162,7 +162,19 @@ void UserServiceAnnChannel::workerLoop() m_announcementChannelRunning = true; #define CHECK_CANCEL_MS 100 + // BUG FIX (found live, 2026-08-11): sendMbstfRequests() used to be called exactly once + // (guarded by requested_mbstf_dist_session, which was never reset), and the wait below had + // no deadline of its own -- if MBSTF's response to that single request was ever lost + // (confirmed live via gdb: this worker thread stuck forever in the wait_for() below, with + // a real, established TCP connection to MBSTF sitting idle -- some transient SBI/SCP + // hiccup around the same moment, not a deadlock or a bug in the wait itself), the + // announcement channel's distribution session would never be created and NO Service + // Announcement content would ever be pushed, for the lifetime of the MBSF process, with no + // way to recover short of restarting it. Retry after a bounded number of wait iterations + // instead of waiting on the same request forever. +#define MBSTF_DIST_SESSION_RETRY_AFTER_ITERATIONS (5000 / CHECK_CANCEL_MS) /* 5 seconds */ bool requested_mbstf_dist_session = false; + unsigned mbstf_dist_session_wait_iterations = 0; std::lock_guard lock(*m_announcementChannelMutex); while (true) { @@ -181,9 +193,16 @@ void UserServiceAnnChannel::workerLoop() ogs_debug("Request creation of USAC MBSTF Dist Session"); m_userServiceAnnChannelDataIngSession->sendMbstfRequests(); requested_mbstf_dist_session = true; + mbstf_dist_session_wait_iterations = 0; } if (!m_userServiceAnnChannelDataIngSession->hasMbstfResponded(USER_SERVICE_ANN_CHANNEL)) { + if (++mbstf_dist_session_wait_iterations >= MBSTF_DIST_SESSION_RETRY_AFTER_ITERATIONS) { + ogs_warn("No response from MBSTF for USAC Dist Session after %u ms -- retrying", + mbstf_dist_session_wait_iterations * CHECK_CANCEL_MS); + requested_mbstf_dist_session = false; + continue; + } // dist session not present, wait for change ogs_debug("Wait for USAC MBSTF Dist Session"); m_announcementChannelChange.wait_for(*m_announcementChannelMutex, std::chrono::milliseconds(CHECK_CANCEL_MS)); From b2aa23e7ab4a848a60a7aa6d4295025ad12a5306 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 16:59:31 +0200 Subject: [PATCH 09/15] Fix uncaught-exception crash on a malformed/misrouted mbs-user-services POST The POST branch of the /nmbsf-mbs-us/v1/mbs-user-services dispatch matched on resource0 == "mbs-user-services" alone, without checking that there was no sub-resource after it -- so a request actually meant for a sub-resource path (e.g. a client hitting /mbs-user-services/{id}/ingest-sessions instead of the real /nmbsf-mbs-ud-ingest/v1/sessions endpoint) got parsed as if it were a brand-new MBSUserService creation body instead. Confirmed live: that body is missing fields MBSUserService's constructor requires (e.g. extServiceIds), and checkAndSetUserServiceAnnouncementChannel() constructs a raw MBSUserService from it with no try/catch of its own, so the resulting fiveg_mag_reftools::ModelException was uncaught, called std::terminate(), and took the whole MBSF process down -- instead of the 400 Bad Request a malformed or misrouted client request should get. Two independent fixes: only match the POST-creates-a-new-service case when there is no resource1 (mbs-user-services is a collection endpoint, so a real create request never has one), and wrap checkAndSetUserServiceAnnouncementChannel() in a try/catch so a genuinely malformed but correctly-routed body (missing a required field) gets a proper error response instead of crashing the process either way. mbsf.yaml.in: cross-reference comment on userServiceAnnouncement pointing at rt-mbs-client's new static announcement-channel bootstrap config, which must agree with ssmDestinationAddress/ssmPort/the hardcoded announcement TSI here. --- src/mbsf/UserService.cc | 31 ++++++++++++++++++++++++++----- src/mbsf/mbsf.yaml.in | 7 +++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/mbsf/UserService.cc b/src/mbsf/UserService.cc index 0fe2a5b..d6f50a5 100644 --- a/src/mbsf/UserService.cc +++ b/src/mbsf/UserService.cc @@ -226,7 +226,16 @@ bool UserService::processEvent(Open5GSEvent &event) if (resource0 == "mbs-user-services") { std::string method(message.method()); const char *ptr_resource1 = message.resourceComponent(1); - if (method == OGS_SBI_HTTP_METHOD_POST) { + // BUG FIX: this POST branch used to match any "mbs-user-services" prefix regardless + // of what followed it, so a request actually meant for a sub-resource (e.g. a + // misrouted or malformed "/mbs-user-services/{id}/ingest-sessions") would be parsed + // as if it were a brand-new MBSUserService creation body instead -- confirmed live: + // that body is missing fields MBSUserService requires (e.g. extServiceIds), and + // checkAndSetUserServiceAnnouncementChannel() below constructs a raw MBSUserService + // from it with no try/catch, so the resulting ModelException was uncaught and took + // the whole process down. A real POST to create a user service has no resource1 at + // all (mbs-user-services is a collection endpoint); only match that case here. + if (method == OGS_SBI_HTTP_METHOD_POST && !ptr_resource1) { ogs_debug("POST response: status = %i", message.resStatus()); std::shared_ptr user_service; ogs_debug("Request body: %s", request.content()); @@ -249,11 +258,23 @@ bool UserService::processEvent(Open5GSEvent &event) return true; } - if(!checkAndSetUserServiceAnnouncementChannel(mbs_user_service, true)) { - static const char *err = "MBSF cannot handle User Service Announcement channel without local configuration."; - ogs_error("%s", err); + // BUG FIX: checkAndSetUserServiceAnnouncementChannel() constructs a raw MBSUserService + // straight from the request body (fiveg_mag_reftools::ModelException on any missing + // required field, e.g. extServiceIds) with no try/catch of its own -- confirmed live: + // an uncaught ModelException here calls std::terminate() and takes the whole MBSF + // process down, rather than the 400 Bad Request a malformed client body should get. + try { + if(!checkAndSetUserServiceAnnouncementChannel(mbs_user_service, true)) { + static const char *err = "MBSF cannot handle User Service Announcement channel without local configuration."; + ogs_error("%s", err); + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_BAD_REQUEST, 1, message, + app_meta, api, "Bad MBSF User Service", err)); + return true; + } + } catch (const std::exception &ex) { + ogs_error("Malformed MBS User Service in request body: %s", ex.what()); ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_BAD_REQUEST, 1, message, - app_meta, api, "Bad MBSF User Service", err)); + app_meta, api, "Bad MBSF User Service", ex.what())); return true; } diff --git a/src/mbsf/mbsf.yaml.in b/src/mbsf/mbsf.yaml.in index 53553b1..3a0ab21 100644 --- a/src/mbsf/mbsf.yaml.in +++ b/src/mbsf/mbsf.yaml.in @@ -31,6 +31,13 @@ mbsf: mbsUserServiceMaxAge: 60 mbsUserDataIngestSessionMaxAge: 60 + # NOTE: rt-mbs-function does not implement a real MBS-5 (TS 26.517 cl.9.2) discovery API or any + # MBS-4-MC bootstrap signalling, so a client has no protocol-level way to learn where the + # announcement channel actually is. If rt-mbs-client's mbsf_client.announcement_channel config + # (rt-mbs-client.conf) is used to bootstrap it automatically instead of a manual ManualActivate + # call, its multicast_address/port must match ssmDestinationAddress/ssmPort below, and its tsi + # must match the announcement channel's TSI (hardcoded to 1 in UserDataIngSession.cc -- see + # g_next_tsi there, which starts real content sessions at 2 to leave 1 free for this). userServiceAnnouncement: announcementRepetitionTime: 10000 ssmPort: 3000 From d479af76ae689d0377e55b45795575e58efe9f52 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 18:01:27 +0200 Subject: [PATCH 10/15] Fix Service Announcement channel using a random port instead of the configured one The announcement-channel-specific branch of UserDataIngSession's distribution session setup drew a fresh random port (32768-65535) for its SSM the same way the regular per-content-session branch does -- correct there (a new content session legitimately gets a new port every time), wrong here: the Service Announcement channel is meant to be a single, fixed, well-known channel, which is exactly why mbsf.yaml already has a userServiceAnnouncement.ssmPort config value and rt-mbs-client has a matching static mbsf_client.announcement_channel bootstrap config -- ssmPort was already correctly plumbed through Context (Context::userServiceAnnSsmPort()), it just wasn't being used for this. Confirmed live via tcpdump on the UE's own TUN device: with the random port, MBSTF genuinely transmitted the real FLUTE carousel content on some other, unpredictable port every run (e.g. 41873) while a client statically configured with ssmPort's value (3000) filtered every real packet out silently, since the destination port never matched -- despite the PDCP/RLC/GW chain now correctly delivering the content to the TUN device (confirmed by the prior PDCP SN-size fix in srsRAN_Project_mbs). This made the whole reproducible-bootstrap design pointless: the one value it depended on being fixed wasn't actually fixed. Fix: use App::self().context()->userServiceAnnSsmPort() for the announcement channel's own SSM port instead of the random generator. Builds clean. --- src/mbsf/UserDataIngSession.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index 6949a84..be9cb91 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -1155,9 +1155,20 @@ void UserDataIngSession::userServiceAnnChannelDistributionSessionInfo() const std::optional &dest_ipv4_addr = dest_ip_addr->getIpv4Addr(); const std::optional> &dest_ipv6_addr = dest_ip_addr->getIpv6Addr(); std::shared_ptr ssm_data(new Ssm(*ssm_val)); - static std::random_device rd; - static std::uniform_int_distribution ud(32768, 65535); - in_port_t port = ud(rd); + // BUG FIX: this used to draw a fresh random port (ud(rd), the same + // generator the regular per-content-session branch above uses, where a + // new port every session is genuinely correct) for the Service + // Announcement channel too -- but the announcement channel is meant to + // be a single, fixed, well-known channel a client can bootstrap from + // static configuration (see mbsf.yaml's userServiceAnnouncement.ssmPort, + // and rt-mbs-client.conf's matching mbsf_client.announcement_channel). + // Confirmed live: with the random port, MBSTF ended up transmitting the + // real FLUTE carousel on some other, unpredictable port every run (e.g. + // 41873), while any client bootstrapped from the configured ssmPort + // (3000) filtered every real packet out silently, since it never matches + // -- ssmPort was already correctly plumbed through Context (see + // Context::userServiceAnnSsmPort()), it just wasn't used here. + in_port_t port = static_cast(App::self().context()->userServiceAnnSsmPort()); uint64_t tsi = 0; if (info->getDistrMethod()->getValue() == DistributionMethod::VAL_OBJECT) { tsi = 1; From 9674b541ba6ee0e9fa48265371ee47014d57b1bd Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 18:58:01 +0200 Subject: [PATCH 11/15] Fix MBSF crash: unbounded retry loop when MBSTF rejects a distribution session createMbsSession() unconditionally built a brand new MBSMFMBSSession (and the underlying mb_smf_sc_mbs_session_new_ipv4()/_ipv6() C session object) on every call, only guarding the *assignment* to context_data->MBSSession ('if (!context_data->MBSSession)' further down) rather than the work itself. isMBSSessionCreated() only flips true once the underlying MB-SMF session genuinely reaches CREATED state -- if MBSTF ever rejects the distribution session (confirmed live: a malformed request), that never happens, and userServiceAnnChannelDistributionSessionInfo()'s periodic check ('if (!isMBSSessionCreated(key)) createMbsSession(...)') called this again, immediately, every single loop iteration, forever: no backoff, no bound. Confirmed live: MBSF spun at the announcement-channel workerLoop's tick rate (tens of iterations/second) reconstructing the C session object and re-notifying MB-SMF each time, until it crashed. Fix: skip entirely once a session object for this context already exists. The caller's own retry-driving state (MBSSessionStatus, receivedMBSTFResponse) is what should progress it from here, not another blind rebuild. Builds clean. --- src/mbsf/UserDataIngSession.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index be9cb91..5aa8e68 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -1663,6 +1663,24 @@ bool UserDataIngSession::handleMbstfDiscover(ogs_sbi_nf_instance_t *nf_instance, bool UserDataIngSession::createMbsSession(const std::shared_ptr &context_data) { + // BUG FIX: this function used to unconditionally build a brand new MBSMFMBSSession (and the + // underlying mb_smf_sc_mbs_session_new_ipv4()/_ipv6() C session object) on every call, only + // guarding the *assignment* to context_data->MBSSession ("if (!context_data->MBSSession)" + // below) rather than the work itself. isMBSSessionCreated() only flips to true once the + // underlying MB-SMF session genuinely reaches CREATED state -- if MBSTF ever rejects the + // distribution session (e.g. a malformed request), that never happens, and + // userServiceAnnChannelDistributionSessionInfo()'s periodic check + // ("if (!isMBSSessionCreated(key)) createMbsSession(...)") called this again, immediately, + // every single loop iteration, forever: no backoff, no bound. Confirmed live -- MBSF spun at + // the workerLoop's tick rate (dozens of iterations/second) reconstructing the C session object + // and re-notifying MB-SMF each time, until it crashed. Skip entirely once a session object for + // this context already exists; the caller's own retry-driving state (MBSSessionStatus, + // receivedMBSTFResponse) is what should progress it from here, not another blind rebuild. + if (context_data->MBSSession) { + ogs_debug("createMbsSession: MBS Session already exists for this context, not recreating"); + return true; + } + const auto &ssm_ptr = context_data->ssm; if (!ssm_ptr) ogs_error("Unable to get SSM from Context Data"); From 69a0ece0d7d7ac75e8e1fae97f944266c1470033 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 20:02:41 +0200 Subject: [PATCH 12/15] Fix distribution session state-change PATCH: wrong classification + missing wrapper Two compounding bugs blocked every activate/deactivate on an already-created distribution session: 1. UserDataIngSession's update-merge loop compared the full MBSDistributionSessionInfo (including mbsDistSessState) to decide needsUpdate vs the lightweight stateUpdate path. Since this API has no separate state-only endpoint, every activate/deactivate PUT changed state alongside resending the rest of the body, so it always (mis)classified as needsUpdate -- triggering a full session rebuild PATCH instead of the purpose-built stateUpdate PATCH. 2. That needsUpdate PATCH path itself was also broken: it sent the DistSession's JSON directly as the patch value for an empty-path add/replace, but MBSTF's actual patch target for /dist-sessions/{id} is a CreateReqData, whose fromJSON() requires the value to be a full CreateReqData document (a distSession key wrapping the fields) -- confirmed live via MBSTF's own error: 'Mandatory Information Element Missing: distSession: Field "distSession" is required'. Fixed both: state-only changes now correctly route through the lightweight stateUpdate path (state normalised out before the equality check), and the needsUpdate path now wraps its patch value correctly. Verified live: PUT to ACTIVE on a real pushed-content distribution session succeeded and MBSTF actually transmitted the object, which previously failed outright. --- src/mbsf/Nmb2Build.cc | 17 ++++++++++++++--- src/mbsf/UserDataIngSession.cc | 29 ++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/mbsf/Nmb2Build.cc b/src/mbsf/Nmb2Build.cc index 12987f0..17a98fc 100644 --- a/src/mbsf/Nmb2Build.cc +++ b/src/mbsf/Nmb2Build.cc @@ -242,8 +242,6 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat std::shared_ptr context_data_ptr(ing_session->getDistributionSessionInfoData(session_ids->second->second)); DistSessionState req_state; if (context_data_ptr->needsUpdate) { - // TS 29.581: PATCH /dist-sessions/{distSessionRef} operates on the flat - // DistSession resource directly, with no "distSession" wrapper property. // RFC 6901: the whole document is addressed by the empty JSON Pointer "". status_item.path = (char *)""; std::shared_ptr dist_session = build_nmb2_create_dist_session(ing_session, context_data_ptr); @@ -253,7 +251,20 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat dist_session->setDistSessionId(sess_id); UserDataIngSession::addToRegistry(sess_id, session_ids->second); - patch_val = dist_session->toJSON(true); + // BUG FIX (found live, 2026-08-11): MBSTF stores the PATCH target for + // /dist-sessions/{id} as a CreateReqData (see DistributionSession.cc, which patches + // distributionSessionReqData() -- a CreateReqData, not a bare DistSession), and + // CreateReqData::fromJSON() (invoked by its applyJSONPatch() for an empty-path + // add/replace) requires its value to be a full CreateReqData document -- i.e. an + // object with a "distSession" property wrapping the DistSession fields, not the + // DistSession's own JSON directly. Sending dist_session->toJSON() unwrapped, as this + // used to, made every needsUpdate PATCH (a content/session change, not just a state + // change -- see the stateUpdate branch below for that) fail with "Mandatory + // Information Element Missing: distSession: Field \"distSession\" is required", + // silently breaking updates to any already-created distribution session. + CJson wrapped_patch_val = CJson::newObject(); + wrapped_patch_val.set("distSession", dist_session->toJSON(true)); + patch_val = wrapped_patch_val; const auto &state = dist_session->getDistSessionState(); if (state) req_state = *state; } else if (context_data_ptr->stateUpdate) { diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index 5aa8e68..e5649d7 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -1273,9 +1273,36 @@ void UserDataIngSession::processUserDataIngSessionUpdate(ogs_pool_id_t stream_id update_info->setMbsSessionId(info->getMbsSessionId()); update_info->setLocationDependent(info->getLocationDependent()); - if (*update_info != *info) { + // BUG FIX (found live, 2026-08-11): mbsDistSessState is included in + // MBSDistributionSessionInfo::operator!=, so a PUT that changes ONLY the + // state (activate/deactivate -- the common case, since this API has no + // separate lightweight state-only endpoint) always took the needsUpdate + // branch below, which rebuilds and PATCHes the ENTIRE MBSTF distribution + // session, instead of the purpose-built, lightweight stateUpdate path (see + // setDistSessionState() / buildNmb2DistSessionPatch()'s + // "/distSession/distSessionState" branch) meant for exactly this. Compare + // with state normalised out first, so pure state changes are classified + // as stateUpdate, not needsUpdate; a change to anything else still counts + // as needsUpdate regardless of whether state also changed (the needsUpdate + // path's rebuilt DistSession already carries the new state along with it). + const auto orig_update_state = update_info->getMbsDistSessState(); + update_info->setMbsDistSessState(info->getMbsDistSessState()); + bool content_changed = (*update_info != *info); + update_info->setMbsDistSessState(orig_update_state); + + if (content_changed) { context_data->needsUpdate = true; context_data->distributionSessionInfo->updateMBSDistributionSessionInfo(update_info); + } else if (orig_update_state != info->getMbsDistSessState()) { + context_data->stateUpdate = true; + info->setMbsDistSessState(orig_update_state); + // buildNmb2DistSessionPatch()'s stateUpdate branch reads the wanted + // state off context_data->info, which is normally the same object as + // this loop's info -- set both explicitly rather than relying on that + // aliasing. + if (context_data->info && context_data->info != info) { + context_data->info->setMbsDistSessState(orig_update_state); + } } } update_dist_sess_infos.erase(key_in_update); From 5a3ae807d8b4355585de80a371af6f4af5be3e31 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 21:03:00 +0200 Subject: [PATCH 13/15] Fix MBSF process-wide crash on invalid objDistrInfo update processUserDataIngSessionUpdate()'s caller only caught std::out_of_range, not ModelException. updateMBSDistributionSessionInfo() correctly throws a ModelException when a client PATCHes objDistrInfo/pckDistrInfo while the Distribution Session isn't INACTIVE (a real, intentional validation, not a bug) -- but with no catch for that type, the exception propagated all the way out of the SBI request handler uncaught and crashed the entire MBSF process via std::terminate(), taking down every other active session with it over a single bad client request. Confirmed live: PATCHing objAcqIds on an ACTIVE session crashed MBSF outright. Fixed by catching ModelException here too and converting it to a proper error response, same pattern already used for the actPeriods/actPeriodsRepRule validation a few lines above. --- src/mbsf/UserDataIngSession.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index e5649d7..c68cf51 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -534,6 +534,18 @@ bool UserDataIngSession::processEvent(Open5GSEvent &event) ogs_assert(true == Open5GSSBIServer::sendResponse(stream, *response)); } catch (const std::out_of_range &e) { send_invalid_user_data_ing_session_err(e, stream, 3, message, app_meta, api, user_data_ing_session_id); + } catch (ModelException &ex) { + // BUG FIX (found live, 2026-08-11): processUserDataIngSessionUpdate() + // (via updateMBSDistributionSessionInfo()) throws a ModelException for + // a genuinely invalid update -- e.g. PATCHing objDistrInfo/pckDistrInfo + // while the Distribution Session isn't INACTIVE (correctly rejected, + // not a bug in itself) -- but nothing here caught it, so it propagated + // all the way out of the SBI request handler uncaught and crashed the + // entire MBSF process via std::terminate(), taking down every other + // active session with it over a single bad client request. Convert it + // to a proper error response instead, same pattern as the + // actPeriods/actPeriodsRepRule validation above. + send_model_error(ex, stream, 3, message, app_meta, api, "Problem with UserDataIngSession update", "Applying UserDataIngSession update"); } return true; From 37048771b684b8b1cbc809be82f414e348756ee1 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 11 Aug 2026 21:48:21 +0200 Subject: [PATCH 14/15] Bump rt-common-shared submodule: fix CJson copy-assignment SIGILL crash --- subprojects/rt-common-shared | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/rt-common-shared b/subprojects/rt-common-shared index 1c6ffdf..25f848f 160000 --- a/subprojects/rt-common-shared +++ b/subprojects/rt-common-shared @@ -1 +1 @@ -Subproject commit 1c6ffdf20e6b10b932402d66e2214dd88f6bde43 +Subproject commit 25f848fdf07cf918756b685bc9bd881b1d50aecf From 02ea93a0d5db8b2ac99b177ed800c2b7e932dc91 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Wed, 12 Aug 2026 08:54:06 +0200 Subject: [PATCH 15/15] Fix MBS Session ID (SSM address) leak on MBS User Service deletion Context::addMbsSessionId()/deleteMbsSessionId() track SSM addresses currently in use, keyed by a UniqueMbsSessionId built from the SSM plus service area info. Both of the two deletion-completion call sites that were supposed to release this on delete passed the wrong key type: - setMBSSessionDeleted() passed ids.second (the distSessionInfoKey, e.g. "AP_MBS_SESSION_1") to removeFromRegistry() -- which is keyed by the real MBSTF-assigned distribution session ID -- and ids.first (the ingSessionId, a UUID) to removeDistributionSessionInfo() -- which is keyed by distSessionInfoKey. Both erase()-by-wrong-key calls silently no-op. - setMBSTFDistSessionDeletedFlag()'s whole cleanup block was commented out, and its own draft had the identical ids->first/ids->second mismatch, so re-enabling it as originally written would have no-op'd the same way. Net effect, confirmed live: a deleted MBS User Service's SSM address stayed registered in Context::m_mbsSessionIds forever, so recreating a session on the same address (e.g. any repeated test run, or any deployment that reuses a small SSM pool) logs a permanent "Attempt to insert duplicate UniqueMBSSessionId" warning and leaks a UserDataIngSession object every time. Fixed both call sites to use ContextData's own correctly-populated fields (mbstfDistSessionId, distSessionInfoKey) instead of the mismatched ids pair. Re-enabling setMBSTFDistSessionDeletedFlag()'s full cleanup (including the final deleteUserDataIngSession() call) also had to be reverted after live testing surfaced a double-deletion race with setMBSSessionDeleted() -- both fire for the same logical session on a normal delete, and both would try to tear down the same UserDataIngSession, destroying it before setMBSSessionDeleted()'s own deferred DELETE HTTP response could be sent ("User Data Ingest Session deleted before N pending responses sent", then the portal's request timing out). setMBSSessionDeleted() remains the sole trigger for the final teardown; setMBSTFDistSessionDeletedFlag() now only does its own partial-removal (markForDeletion) and registry-cleanup duties. Verified live: 3 consecutive create+delete cycles on the same SSM address produce no duplicate-registration warnings and no deletion errors, and the full MBS Broadcast Service Announcement flow (verify-e2e.sh) still completes cleanly end to end afterwards. --- src/mbsf/UserDataIngSession.cc | 55 +++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/mbsf/UserDataIngSession.cc b/src/mbsf/UserDataIngSession.cc index c68cf51..821076b 100644 --- a/src/mbsf/UserDataIngSession.cc +++ b/src/mbsf/UserDataIngSession.cc @@ -2186,8 +2186,20 @@ void UserDataIngSession::setMBSSessionDeleted(const UserDataIngDistSessId &ids) } ing_sess->m_deleteRequests.clear(); if (context_data->markForDeletion) { - removeFromRegistry(ids.second); - ing_sess->removeDistributionSessionInfo(ids.first); + // BUG FIX (found live, 2026-08-12): these used to pass ids.second (the + // distSessionInfoKey, e.g. "AP_MBS_SESSION_1") to removeFromRegistry() -- which + // is keyed by the real MBSTF-assigned distribution session ID -- and ids.first + // (the ingSessionId, a UUID) to removeDistributionSessionInfo() -- which is keyed + // by distSessionInfoKey. Both erase()-by-wrong-key calls silently no-op (no + // exception, no log), so s_distSessionIdRegistry and m_distributionSessionInfos + // were never actually cleared here. Confirmed live: this left the deleted + // session's MbsSessionId (and thus its SSM address) permanently registered in + // Context::m_mbsSessionIds (see Context::addMbsSessionId's "Attempt to insert + // duplicate" warning), reproducible on every create+delete+recreate cycle with + // the same SSM, even minutes apart. Fixed to use context_data's own, correctly + // populated fields instead of the mismatched ids pair. + removeFromRegistry(context_data->mbstfDistSessionId); + ing_sess->removeDistributionSessionInfo(context_data->distSessionInfoKey); } App::self().context()->deleteUserDataIngSession(ing_sess->m_UserDataIngSessionId); } @@ -2255,15 +2267,38 @@ void UserDataIngSession::setMBSTFDistSessionDeletedFlag(const std::string &dist_ ogs_debug("Deleting MBS Session for Dist Session %s", dist_session_id.c_str()); context_data->MBSSession->deleteSession(); } - //if (context_data->markForDeletion) { - //removeFromRegistry(dist_session_id); - //ing_sess->removeDistributionSessionInfo(ids->first); - //return; - //} + // BUG FIX (found live, 2026-08-12): this whole cleanup was commented out, and the + // original draft passed ids->first (the ingSessionId, a UUID) to + // removeDistributionSessionInfo() -- which is keyed by distSessionInfoKey (e.g. + // "AP_MBS_SESSION_1") -- so even re-enabled as-is it would have silently no-op'd (see + // the identical bug fixed in setMBSSessionDeleted() above). With nothing here to ever + // erase this session from s_distSessionIdRegistry/m_distributionSessionInfos or drop the + // UserDataIngSession's last shared_ptr reference, a deleted MBS User Service's + // MbsSessionId (and its SSM address) stayed registered in Context::m_mbsSessionIds + // forever once its Distribution Session's deletion was confirmed via this path. + // Confirmed live via Context::addMbsSessionId's "Attempt to insert duplicate" warning, + // reproducible on every create+delete+recreate cycle using the same SSM. dist_session_id + // (this function's own parameter) is already the correct registry key -- no field + // lookup needed for that one. + if (context_data->markForDeletion) { + removeFromRegistry(dist_session_id); + ing_sess->removeDistributionSessionInfo(context_data->distSessionInfoKey); + return; + } } - //if (ing_sess->checkIfAllMBSTFDistSessionDeleted()) { - // App::self().context()->deleteUserDataIngSession(ing_sess->m_UserDataIngSessionId); - //} + // NOT calling deleteUserDataIngSession() here (found live, 2026-08-12): setMBSSessionDeleted() + // is the authoritative "whole ingest session torn down" trigger -- it already sends the + // deferred DELETE HTTP response(s) queued in m_deleteRequests before destroying the session, + // gated on checkIfAllMBSSessionDeletionsReceived(). A normal delete fires BOTH this function + // (MBSTF distribution session deleted) and setMBSSessionDeleted() (MB-SMF MBS Session deleted) + // for the same logical session; re-enabling this call as well raced the two paths and + // destroyed the UserDataIngSession from here first, before setMBSSessionDeleted() could send + // its queued response -- confirmed live: "User Data Ingest Session deleted before 1 pending + // responses sent" followed by the portal's DELETE request timing out after 30s waiting for a + // response that could now never be sent. checkIfAllMBSTFDistSessionDeleted() still runs, for + // its s_distSessionIdRegistry cleanup side effect, but its own deleteUserDataIngSession() call + // stays commented out (see the function itself) for the same reason. + ing_sess->checkIfAllMBSTFDistSessionDeleted(); } void UserDataIngSession::populateAndSendError(UserDataIngDistSessId *ids, const std::optional &cause, const std::optional &problem_detail_json)