Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
lynxis lazus has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. sgsn_mm_ctx_alloc(): check for unallocated fsms Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 --- M src/sgsn/gprs_sgsn.c 1 file changed, 25 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/sgsn/gprs_sgsn.c b/src/sgsn/gprs_sgsn.c index 474f8f9..cb2c0fc 100644 --- a/src/sgsn/gprs_sgsn.c +++ b/src/sgsn/gprs_sgsn.c @@ -243,17 +243,42 @@ talloc_free(ctx); return NULL; } + ctx->gmm_fsm = osmo_fsm_inst_alloc(_fsm, ctx, ctx, LOGL_DEBUG, "gmm_fsm"); + if (!ctx->gmm_fsm) + goto out; ctx->gmm_att_req.fsm = osmo_fsm_inst_alloc(_attach_req_fsm, ctx, ctx, LOGL_DEBUG, "gb_gmm_req"); + if (!ctx->gmm_att_req.fsm) + goto out; ctx->gb.mm_state_fsm = osmo_fsm_inst_alloc(_state_gb_fsm, ctx, ctx, LOGL_DEBUG, NULL); + if (!ctx->gb.mm_state_fsm) + goto out; #ifdef BUILD_IU ctx->iu.mm_state_fsm = osmo_fsm_inst_alloc(_state_iu_fsm, ctx, ctx, LOGL_DEBUG, NULL); + if (!ctx->iu.mm_state_fsm) + goto out; #endif + INIT_LLIST_HEAD(>pdp_list); llist_add(>list, _mm_ctxts); return ctx; + +out: + if (ctx->iu.mm_state_fsm) + osmo_fsm_inst_free(ctx->iu.mm_state_fsm); + if (ctx->gb.mm_state_fsm) + osmo_fsm_inst_free(ctx->gb.mm_state_fsm); + if (ctx->gmm_att_req.fsm) + osmo_fsm_inst_free(ctx->gmm_att_req.fsm); + if (ctx->gmm_fsm) + osmo_fsm_inst_free(ctx->gmm_fsm); + + rate_ctr_group_free(ctx->ctrg); + talloc_free(ctx); + + return NULL; } /* Allocate a new SGSN MM context for GERAN_Gb */ struct sgsn_mm_ctx *sgsn_mm_ctx_alloc_gb(uint32_t tlli, -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: pespin Gerrit-MessageType: merged
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 08 Oct 2019 20:38:32 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c File src/sgsn/gprs_sgsn.c: https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c@270 PS1, Line 270: osmo_fsm_inst_free(ctx->iu.mm_state_fsm); > it does not guard against it. it would be worth a separate change to libosmocore to fix that. However, applications still must have their own null-checks for the time being, as long as they still work with older released libosmocore versions :/ -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Comment-Date: Tue, 08 Oct 2019 20:38:19 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: lynxis lazus Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 08 Oct 2019 16:59:24 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
lynxis lazus has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c File src/sgsn/gprs_sgsn.c: https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c@270 PS1, Line 270: osmo_fsm_inst_free(ctx->iu.mm_state_fsm); > I'd expect osmo_fsm_inst_free() to guard against NULL pointers (free > functions usually do). […] it does not guard against it. -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus Gerrit-CC: pespin Gerrit-Comment-Date: Tue, 08 Oct 2019 16:53:24 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 ) Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c File src/sgsn/gprs_sgsn.c: https://gerrit.osmocom.org/#/c/15704/1/src/sgsn/gprs_sgsn.c@270 PS1, Line 270: osmo_fsm_inst_free(ctx->iu.mm_state_fsm); I'd expect osmo_fsm_inst_free() to guard against NULL pointers (free functions usually do). Can you check it? -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Comment-Date: Tue, 08 Oct 2019 15:34:18 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in ...osmo-sgsn[master]: sgsn_mm_ctx_alloc(): check for unallocated fsms
lynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 Change subject: sgsn_mm_ctx_alloc(): check for unallocated fsms .. sgsn_mm_ctx_alloc(): check for unallocated fsms Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 --- M src/sgsn/gprs_sgsn.c 1 file changed, 25 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/04/15704/1 diff --git a/src/sgsn/gprs_sgsn.c b/src/sgsn/gprs_sgsn.c index 474f8f9..cb2c0fc 100644 --- a/src/sgsn/gprs_sgsn.c +++ b/src/sgsn/gprs_sgsn.c @@ -243,17 +243,42 @@ talloc_free(ctx); return NULL; } + ctx->gmm_fsm = osmo_fsm_inst_alloc(_fsm, ctx, ctx, LOGL_DEBUG, "gmm_fsm"); + if (!ctx->gmm_fsm) + goto out; ctx->gmm_att_req.fsm = osmo_fsm_inst_alloc(_attach_req_fsm, ctx, ctx, LOGL_DEBUG, "gb_gmm_req"); + if (!ctx->gmm_att_req.fsm) + goto out; ctx->gb.mm_state_fsm = osmo_fsm_inst_alloc(_state_gb_fsm, ctx, ctx, LOGL_DEBUG, NULL); + if (!ctx->gb.mm_state_fsm) + goto out; #ifdef BUILD_IU ctx->iu.mm_state_fsm = osmo_fsm_inst_alloc(_state_iu_fsm, ctx, ctx, LOGL_DEBUG, NULL); + if (!ctx->iu.mm_state_fsm) + goto out; #endif + INIT_LLIST_HEAD(>pdp_list); llist_add(>list, _mm_ctxts); return ctx; + +out: + if (ctx->iu.mm_state_fsm) + osmo_fsm_inst_free(ctx->iu.mm_state_fsm); + if (ctx->gb.mm_state_fsm) + osmo_fsm_inst_free(ctx->gb.mm_state_fsm); + if (ctx->gmm_att_req.fsm) + osmo_fsm_inst_free(ctx->gmm_att_req.fsm); + if (ctx->gmm_fsm) + osmo_fsm_inst_free(ctx->gmm_fsm); + + rate_ctr_group_free(ctx->ctrg); + talloc_free(ctx); + + return NULL; } /* Allocate a new SGSN MM context for GERAN_Gb */ struct sgsn_mm_ctx *sgsn_mm_ctx_alloc_gb(uint32_t tlli, -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15704 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I867612a60236eaf7009400c92f5d871006aaf008 Gerrit-Change-Number: 15704 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus Gerrit-MessageType: newchange
