On Thu, Oct 13, 2016 at 09:32:21AM -0700, scan-admin@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