Coverity issues in lapd_core/gsm mncc

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/OpenBSC@lists.osmocom.org/.

Holger Hans Peter Freyther holger at freyther.de
Fri Jul 5 05:43:15 UTC 2013


Dear Andreas,

the coverity prevent utility has found three inconsistencies in the
lapd_core.c and the gsm_04_08.c and I think you are suited the best
to comment on how to resolve them.


src/gsm/lapd_core.c
static int lapd_res_req(struct osmo_dlsap_prim *dp, struct lapd_msg_ctx *lctx)
...
1943        LOGP(DLLAPD, LOGL_INFO, "perform re-establishment (SABM) length=%d\n",
1944                msg->len);
...
 	
CID 1040665 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking "msg" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
1956        if (msg && msg->len)

so we already dereferenced msg to print the length. So at this point,
is it legetimate to a have NULL msgb or shall the null check be removed?


src/libmsc/gsm_04_08.c
static int gsm48_cc_rx_connect(struct gsm_trans *trans, struct msgb *msg)
...
2090        if (trans->subscr) {
2091                connect.fields |= MNCC_F_CONNECTED;
2092                strncpy(connect.connected.number, trans->subscr->extension,
2093                        sizeof(connect.connected.number)-1);
2094                strncpy(connect.imsi, trans->subscr->imsi,
2095                        sizeof(connect.imsi)-1);
2096        }
...
	
CID 1040740 (#1 of 1): Dereference after null check (FORWARD_NULL)
6. var_deref_op: Dereferencing null pointer "trans->subscr".
2117        osmo_counter_inc(trans->subscr->net->stats.call.mt_connect);
2118
2119        return mncc_recvmsg(trans->subscr->net, trans, MNCC_SETUP_CNF, &connect);

trans->subscr is conditionally dereferenced and then unconditionally,
what is correct?


static int gsm48_cc_rx_setup(struct gsm_trans *trans, struct msgb *msg)
...
1761        if (trans->subscr) {
1762                setup.fields |= MNCC_F_CALLING;
1763                strncpy(setup.calling.number, trans->subscr->extension,
1764                        sizeof(setup.calling.number)-1);
1765                strncpy(setup.imsi, trans->subscr->imsi,
1766                        sizeof(setup.imsi)-1);
1767        }
...
CID 1040739 (#1 of 1): Dereference after null check (FORWARD_NULL)
14. var_deref_model: Passing null pointer "trans->subscr" to function "subscr_name(struct gsm_subscriber *)", which dereferences it. [show details]
1813        LOGP(DCC, LOGL_INFO, "Subscriber %s (%s) sends SETUP to %s\n",
1814             subscr_name(trans->subscr), trans->subscr->extension,
1815             setup.called.number);
1816
1817        osmo_counter_inc(trans->subscr->net->stats.call.mo_setup);

Same thing as above. Can the null check be removed? Would you have the
time to do that?




More information about the OpenBSC mailing list