On Thu, Oct 13, 2016 at 09:32:21AM -0700, scan-admin(a)coverity.com wrote:
Hi,
Please find the latest report on new defect(s) introduced to Osmocom found with Coverity
Scan.
4 new defect(s) introduced to Osmocom found with Coverity Scan.
2 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build
analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)
** CID 150135: Null pointer dereferences (REVERSE_INULL)
/source-Osmocom/openggsn/ggsn/ggsn.c: 142 in delete_context()
________________________________________________________________________________________________________
*** CID 150135: Null pointer dereferences (REVERSE_INULL)
/source-Osmocom/openggsn/ggsn/ggsn.c: 142 in delete_context()
136 int delete_context(struct pdp_t *pdp)
137 {
138 DEBUGP(DGGSN, "Deleting PDP context\n");
139 struct ippoolm_t *member = pdp->peer;
140 char v[NAMESIZE];
141 snprintf(v, sizeof(v), "%" PRIu64 ",%s", pdp->imsi,
inet_ntoa(member->addr));
>> CID 150135: Null pointer
dereferences (REVERSE_INULL)
>> Null-checking "pdp->peer" suggests that it may be null, but it
has already been dereferenced on all paths leading to the check.
142 if
(pdp->peer)
143 ippool_freeip(ippool, (struct ippoolm_t *)pdp->peer);
144 else
145 SYS_ERR(DGGSN, LOGL_ERROR, 0, "Peer not defined!");
146
147 if (gtp_kernel_tunnel_del(pdp)) {
This one's for you, Max. (openggsn commit 727417dd28813c697b4820aef9f54f249e30c4b8)
** CID 150134: Null pointer dereferences
(REVERSE_INULL)
/source-Osmocom/openbsc/openbsc/src/gprs/gprs_sgsn.c: 257 in sgsn_mm_ctx_alloc_iu()
________________________________________________________________________________________________________
*** CID 150134: Null pointer dereferences (REVERSE_INULL)
/source-Osmocom/openbsc/openbsc/src/gprs/gprs_sgsn.c: 257 in sgsn_mm_ctx_alloc_iu()
251 ctx->auth_triplet.key_seq = GSM_KEY_SEQ_INVAL;
252 ctx->ctrg = rate_ctr_group_alloc(ctx, &mmctx_ctrg_desc, 0);
253
254 /* Need to get RAID from IU conn */
255 ctx->ra = ctx->iu.ue_ctx->ra_id;
256
fixed (sysmocom/iu branch).
>>
CID 150134: Null pointer dereferences (REVERSE_INULL)
>> Null-checking "ctx->iu.ue_ctx" suggests that it may be null, but
it has already been dereferenced on all paths leading to the check.
257 if
(ctx->iu.ue_ctx)
258 ctx->iu.ue_ctx->rab_assign_addr_enc =
259 sgsn->cfg.iu.rab_assign_addr_enc;
260
261 INIT_LLIST_HEAD(&ctx->pdp_list);
262
** CID 150133: Memory - corruptions (OVERRUN)
/source-Osmocom/osmo-bts/src/common/msg_utils.c: 121 in dtx_cache_payload()
________________________________________________________________________________________________________
*** CID 150133: Memory - corruptions (OVERRUN)
/source-Osmocom/osmo-bts/src/common/msg_utils.c: 121 in dtx_cache_payload()
115 copy_len = OSMO_MIN(length + 1, ARRAY_SIZE(lchan->tch.dtx.cache));
116
117 lchan->tch.dtx.len = copy_len + amr;
118 lchan->tch.dtx.fn = fn;
119 lchan->tch.dtx.is_update = update;
120
>> CID 150133: Memory - corruptions
(OVERRUN)
>> Overrunning buffer pointed to by
"&lchan->tch.dtx.cache[amr]" of 20 bytes by passing it to a function
which accesses it at byte offset 21 using argument "copy_len" (which evaluates
to 20).
121 memcpy(lchan->tch.dtx.cache + amr, l1_payload, copy_len);
@Max, maybe 'copy_len - amr'? In any case there's potentially 'amr'
too many
bytes.
122 }
123
124 /*! \brief Check current state of DTX DL AMR FSM and dispatch necessary events
125 * \param[in] lchan Logical channel on which we check scheduling
126 * \param[in] rtp_pl buffer with RTP data
** CID 150132: Incorrect expression (BAD_SIZEOF)
/source-Osmocom/osmo-bts/src/common/msg_utils.c: 262 in dtx_sched_optional()
________________________________________________________________________________________________________
*** CID 150132: Incorrect expression (BAD_SIZEOF)
/source-Osmocom/osmo-bts/src/common/msg_utils.c: 262 in dtx_sched_optional()
256 h1[] = { 14, 16, 18, 20, 66, 68, 70, 72 };
257 if (lchan->tch_mode == GSM48_CMODE_SPEECH_V1) {
258 if (lchan->type == GSM_LCHAN_TCH_F)
259 return fn_chk(f, fn, ARRAY_SIZE(f));
260 else
261 return fn_chk(lchan->nr ? h1 : h0, fn,
>> CID 150132: Incorrect expression
(BAD_SIZEOF)
>> The expression "sizeof (lchan->nr ? h1 : h0) / sizeof (lchan->nr ?
h1 : h0[0])" is suspicious. Note that "lchan->nr ? h1 : h0" is a pointer
and therefore the division will not return the number of array elements which may have
been the intent.
262 ARRAY_SIZE(lchan->nr ? h1 : h0));
@Max, has to be
lchan-nr? ARRAY_SIZE(h1) : ARRAY_SIZE(h0)
so ARRAY_SIZE can be resolved at compile time.
263 }
264 return false;
265 }
266
267 /* repeat last SID if possible, returns SID length + 1 or 0 */
________________________________________________________________________________________________________
--
- Neels Hofmeyr <nhofmeyr(a)sysmocom.de>
http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschäftsführer / Managing Directors: Harald Welte