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/.
Andreas Eversberg andreas at eversberg.euHolger Hans Peter Freyther wrote: > 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); > ... > this is really wrong. msg may be null. at least it depends on the upper layer how to provide msg (NULL or 0-length), see patch. > > 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? > > also if msg exists with 0 lenght, it will not be used, so it must be freed, see patch. > 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? > i think we can remove the check for trans->subscr, since all rx functions assume that it is set. instead it makes sense to add a sanity check (trans->subscr must be set) to gsm0408_rcv_cc before calling the rx function. > > 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? > same as a bove. -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: lapd.patch URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20130706/5057bd6f/attachment.ksh>