Change in osmo-msc[master]: transaction: fix NULL-pointer dereference in trans_alloc()

[Vadim Yanitskiy]

Vadim Yanitskiy has uploaded this change for review. ( https://gerrit.osmocom.org/13974


Change subject: transaction: fix NULL-pointer dereference in trans_alloc()
......................................................................

transaction: fix NULL-pointer dereference in trans_alloc()

Unlike LOG_TRANS_CAT(), LOG_TRANS() is not NULL-safe:

  LOG_TRANS_CAT(trans, (trans)->log_subsys, level, fmt, ##args)
                       ^^^^^^^^^  NULL-pointer dereference

Let's rather use generic LOGP() macro with pre-calculated logging
subsystem. Intentional use of LOG_TRANS_CAT() doesn't make sense
anyway, because the result would be uninformative:

  trans(NULL NULL callref-0x0 tid-0) ...

Change-Id: I3c373d20ebd6e96ebd57f84b74dc15a6b69c03ac
---
M include/osmocom/msc/transaction.h
M src/libmsc/transaction.c
2 files changed, 6 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/74/13974/1

diff --git a/include/osmocom/msc/transaction.h b/include/osmocom/msc/transaction.h
index 6b82390..a0aeaef 100644
--- a/include/osmocom/msc/transaction.h
+++ b/include/osmocom/msc/transaction.h
@@ -157,11 +157,9 @@
 struct gsm_trans *trans_has_conn(const struct msc_a *msc_a);
 void trans_conn_closed(const struct msc_a *msc_a);
 
-static inline int trans_log_subsys(const struct gsm_trans *trans)
+static inline int trans_log_subsys(enum trans_type type)
 {
-	if (!trans)
-		return DMSC;
-	switch (trans->type) {
+	switch (type) {
 	case TRANS_CC:
 	case TRANS_SILENT_CALL:
 		return DCC;
diff --git a/src/libmsc/transaction.c b/src/libmsc/transaction.c
index d6f8c3b..ebdaced 100644
--- a/src/libmsc/transaction.c
+++ b/src/libmsc/transaction.c
@@ -127,11 +127,12 @@
 			      enum trans_type type, uint8_t trans_id,
 			      uint32_t callref)
 {
-	struct gsm_trans *trans = NULL; /* (NULL for LOG_TRANS() before allocation) */
+	int subsys = trans_log_subsys(type);
+	struct gsm_trans *trans;
 
 	/* a valid subscriber is indispensable */
 	if (vsub == NULL) {
-		LOG_TRANS(trans, LOGL_ERROR, "unable to alloc transaction, invalid subscriber (NULL)\n");
+		LOGP(subsys, LOGL_ERROR, "unable to alloc transaction, invalid subscriber (NULL)\n");
 		return NULL;
 	}
 
@@ -142,11 +143,11 @@
 	*trans = (struct gsm_trans){
 		.vsub = vsub,
 		.type = type,
+		.log_subsys = subsys,
 		.transaction_id = trans_id,
 		.callref = callref,
 		.net = net,
 	};
-	trans->log_subsys = trans_log_subsys(trans);
 	vlr_subscr_get(vsub, trans_vsub_use(type));
 	llist_add_tail(&trans->entry, &net->trans_list);
 

-- 
To view, visit https://gerrit.osmocom.org/13974
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c373d20ebd6e96ebd57f84b74dc15a6b69c03ac
Gerrit-Change-Number: 13974
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190510/7e110ea5/attachment.htm>