On Thu, Oct 13, 2016 at 09:32:21AM -0700, [email protected] 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 <[email protected]> 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
signature.asc
Description: Digital signature
