Change in osmo-bts[master]: osmo-bts-{trx, virtual}: fix: pinst->trx may be NULL

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

fixeria gerrit-no-reply at lists.osmocom.org
Sat May 22 14:19:48 UTC 2021


fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/24286 )

Change subject: osmo-bts-{trx,virtual}: fix: pinst->trx may be NULL
......................................................................

osmo-bts-{trx,virtual}: fix: pinst->trx may be NULL

We assume that it's legal to have dangling PHY instances that are
not associated with any TRX instances in the configuration file.
Obviously, such PHY instances have pinst->trx set to NULL.

The DSP based models seem to handle dangling PHY instances without
any problems, so let's ensure that we always check pinst->trx
against NULL in the osmo-bts-{trx,virtual} specific code.

Change-Id: Ib7d9cb7ae47fead723fa46454cd64bf6e88756bb
Fixes: CID#236092 "Dereference before null check"
---
M include/osmo-bts/phy_link.h
M src/common/scheduler.c
M src/osmo-bts-trx/trx_if.c
M src/osmo-bts-trx/trx_provision_fsm.c
M src/osmo-bts-trx/trx_vty.c
M src/osmo-bts-virtual/l1_if.c
6 files changed, 24 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve
  dexter: Looks good to me, but someone else must approve
  fixeria: Looks good to me, approved



diff --git a/include/osmo-bts/phy_link.h b/include/osmo-bts/phy_link.h
index c4b60f3..becc442 100644
--- a/include/osmo-bts/phy_link.h
+++ b/include/osmo-bts/phy_link.h
@@ -97,7 +97,7 @@
 	struct phy_link *phy_link;
 
 	/* back-pointer to the TRX to which we're associated */
-	struct gsm_bts_trx *trx;
+	struct gsm_bts_trx *trx; /* NOTE: may be NULL! */
 
 	union {
 		struct {
diff --git a/src/common/scheduler.c b/src/common/scheduler.c
index f5a5b89..4af6457 100644
--- a/src/common/scheduler.c
+++ b/src/common/scheduler.c
@@ -578,6 +578,8 @@
 {
 	unsigned int tn, i;
 
+	OSMO_ASSERT(trx != NULL);
+
 	LOGPTRX(trx, DL1C, LOGL_DEBUG, "Init scheduler structures\n");
 
 	for (tn = 0; tn < ARRAY_SIZE(trx->ts); tn++) {
diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index ba30bb2..fcad75c 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -1235,7 +1235,9 @@
 	if (!l1h)
 		return -EINVAL;
 
-	trx_sched_init(pinst->trx);
+	/* PHY instance may be not associated with a TRX instance */
+	if (pinst->trx != NULL)
+		trx_sched_init(pinst->trx);
 
 	rc = trx_if_open(l1h);
 	if (rc < 0) {
diff --git a/src/osmo-bts-trx/trx_provision_fsm.c b/src/osmo-bts-trx/trx_provision_fsm.c
index 1e5de2e..070037d 100644
--- a/src/osmo-bts-trx/trx_provision_fsm.c
+++ b/src/osmo-bts-trx/trx_provision_fsm.c
@@ -279,7 +279,10 @@
 	l1h->config.trxd_pdu_ver_req = pinst->phy_link->u.osmotrx.trxd_pdu_ver_max;
 
 	/* Apply initial RFMUTE state */
-	trx_if_cmd_rfmute(l1h, pinst->trx->mo.nm_state.administrative != NM_STATE_UNLOCKED);
+	if (pinst->trx != NULL)
+		trx_if_cmd_rfmute(l1h, pinst->trx->mo.nm_state.administrative != NM_STATE_UNLOCKED);
+	else
+		trx_if_cmd_rfmute(l1h, true);
 }
 
 static void st_open_poweroff(struct osmo_fsm_inst *fi, uint32_t event, void *data)
diff --git a/src/osmo-bts-trx/trx_vty.c b/src/osmo-bts-trx/trx_vty.c
index b85dcdc..2b0913f 100644
--- a/src/osmo-bts-trx/trx_vty.c
+++ b/src/osmo-bts-trx/trx_vty.c
@@ -99,17 +99,23 @@
 	struct trx_l1h *l1h = pinst->u.osmotrx.hdl;
 	struct gsm_bts_trx *trx = pinst->trx;
 
-	vty_out(vty, "PHY Instance %s%s",
-		phy_instance_name(pinst), VTY_NEWLINE);
+	vty_out(vty, "PHY Instance '%s': bound to %s%s",
+		phy_instance_name(pinst),
+		gsm_trx_name(trx),
+		VTY_NEWLINE);
+
+	if (trx != NULL) {
+		const int actual = get_p_actual_mdBm(trx, trx->power_params.p_total_tgt_mdBm);
+		const int max = get_p_max_out_mdBm(trx);
+		vty_out(vty, " tx-attenuation : %d dB%s",
+			(max - actual) / 1000, VTY_NEWLINE);
+	}
 
 	if (l1h->config.rxgain_valid)
 		vty_out(vty, " rx-gain        : %d dB%s",
 			l1h->config.rxgain, VTY_NEWLINE);
 	else
 		vty_out(vty, " rx-gain        : undefined%s", VTY_NEWLINE);
-	vty_out(vty, " tx-attenuation : %d dB%s",
-		(get_p_max_out_mdBm(trx) - get_p_actual_mdBm(trx, trx->power_params.p_total_tgt_mdBm))/1000,
-		VTY_NEWLINE);
 	if (l1h->config.maxdly_valid)
 		vty_out(vty, " maxdly : %d%s", l1h->config.maxdly,
 			VTY_NEWLINE);
diff --git a/src/osmo-bts-virtual/l1_if.c b/src/osmo-bts-virtual/l1_if.c
index 0fbc35d..444d6ad 100644
--- a/src/osmo-bts-virtual/l1_if.c
+++ b/src/osmo-bts-virtual/l1_if.c
@@ -214,13 +214,15 @@
 
 	/* iterate over list of PHY instances and initialize the scheduler */
 	llist_for_each_entry(pinst, &plink->instances, list) {
+		if (pinst->trx == NULL)
+			continue;
 		trx_sched_init(pinst->trx);
 		/* Only start the scheduler for the transceiver on C0.
 		 * If we have multiple transceivers, CCCH is always on C0
 		 * and has to be auto active */
 		/* Other TRX are activated via OML by a PRIM_INFO_MODIFY
 		 * / PRIM_INFO_ACTIVATE */
-		if (pinst->trx && pinst->trx == pinst->trx->bts->c0) {
+		if (pinst->trx == pinst->trx->bts->c0) {
 			vbts_sched_start(pinst->trx->bts);
 			/* init lapdm layer 3 callback for the trx on timeslot 0 == BCCH */
 			lchan_init_lapdm(&pinst->trx->ts[0].lchan[CCCH_LCHAN]);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/24286
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ib7d9cb7ae47fead723fa46454cd64bf6e88756bb
Gerrit-Change-Number: 24286
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210522/aaa79d18/attachment.htm>


More information about the gerrit-log mailing list