[PATCH 1/4] gprs_gmm: ensure llme present upon Attach Req (CID #57686)

[Neels Hofmeyr]

On Fri, Apr 22, 2016 at 03:47:29PM +0200, Holger Freyther wrote:
> 
> > On 14 Apr 2016, at 15:21, Neels Hofmeyr <nhofmeyr at sysmocom.de> wrote:
> > 
> > ---
> > openbsc/src/gprs/gprs_gmm.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> > 
> > diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
> > index 5f0a5fd..438e047 100644
> > --- a/openbsc/src/gprs/gprs_gmm.c
> > +++ b/openbsc/src/gprs/gprs_gmm.c
> > @@ -838,6 +838,10 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
> > 	int rc;
> > 
> > 	LOGMMCTXP(LOGL_INFO, ctx, "-> GMM ATTACH REQUEST ");
> > +	if (!llme) {
> 
> same as with my other mail. The lower layer should create a llme on the fly. So please have a look at the coverity user interface to see _where_ the llme is NULL and then we can have a look at that issue.

There is, obviously, an explicit NULL on the sysmocom/iu branch, where no LLME
is applicable.

openbsc/openbsc/src/gprs/gprs_gmm.c:2462:		rc = gsm0408_rcv_gmm(mmctx, msg, NULL);


Coverity concerns itself about a

"Comparing llme to null implies that llme might be null."

so no actual NULL is reported. (though coverity could have reported above
explicit NULL which I found manually)

Actually, the llme NULL check in gsm0408_rcv_gmm() is introduced in the
sysmocom/iu branch along with mentioned explicit NULL:

1628        if (llme && !mmctx &&
1629            gh->msg_type != GSM48_MT_GMM_ATTACH_REQ &&
1630            gh->msg_type != GSM48_MT_GMM_RA_UPD_REQ) {
1631                LOGP(DMM, LOGL_NOTICE, "Cannot handle GMM for unknown MM CTX\n");

So my suggestion above to clean up llme only when it's not NULL does still make
a lot of sense to me.

Since I'm not familiar with GRPS code, maybe Daniel could have a quick look?
I'm having a déjà vu, didn't I write this before?
Would be nice if I could stop banging my head against your wall now ;)

~Neels
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20160425/7131c58c/attachment.bin>