diff options
| author | Harald Welte <laforge@gnumonks.org> | 2019-01-20 13:45:31 +0100 | 
|---|---|---|
| committer | Harald Welte <laforge@gnumonks.org> | 2019-01-22 14:53:54 +0000 | 
| commit | 2033be8902f5540f4f0e28bf1b32fb7df0367c66 (patch) | |
| tree | c7b08b1d2993b77fb5f63ce37e8d897ccc6acc52 | |
| parent | 1c3bae138cea1dbde480ce4382120034eb769e82 (diff) | |
Work around bogus gcc-8.2 array-bounds warning/error
gcc-8.2 is printing the following warning, which is an error
when used -Werror like our --enable-werror:
In file included from gprs_bssgp.c:34:
In function ‘tl16v_put’,
    inlined from ‘tvlv_put.part.3’ at ../../include/osmocom/gsm/tlv.h:156:9,
    inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:147:24,
    inlined from ‘msgb_tvlv_push’ at ../../include/osmocom/gsm/tlv.h:386:2,
    inlined from ‘bssgp_tx_dl_ud’ at gprs_bssgp.c:1162:4:
../../include/osmocom/gsm/tlv.h:131:2: error: ‘memcpy’ forming offset [12, 130] is out of the bounds [0, 11] of object ‘mi’ with type ‘uint8_t[11]’ {aka ‘unsigned char[11]’} [-Werror=array-bounds]
  memcpy(buf, val, len);
Where "130" seems to be the maximum value of uint8_t, shifted right one +
2.  But even as we use strnlen() with "16" as maximum upper bound, gcc
still believes there's a way that the return value of gsm48_generate_mid_from_imsi()
could be 130.  In fact, even the newly-added OSMO_ASSERT() inside
gsm48_generate_mid() doesn't help and gcc still insists there is a problem :(
Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec
| -rw-r--r-- | src/gb/gprs_bssgp.c | 14 | ||||
| -rw-r--r-- | src/gb/gprs_bssgp_bss.c | 8 | 
2 files changed, 21 insertions, 1 deletions
diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c index be7ef9f1..4a4bab3e 100644 --- a/src/gb/gprs_bssgp.c +++ b/src/gb/gprs_bssgp.c @@ -1157,10 +1157,17 @@ int bssgp_tx_dl_ud(struct msgb *msg, uint16_t pdu_lifetime,  	/* IMSI */  	if (dup->imsi && strlen(dup->imsi)) {  		uint8_t mi[GSM48_MID_MAX_SIZE]; +/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, + * but somehow gcc (8.2) is not smart enough to figure this out and claims that + * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to + * mi[131], which is wrong */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds"  		int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);  		if (imsi_len > 2)  			msgb_tvlv_push(msg, BSSGP_IE_IMSI,  				imsi_len-2, mi+2); +#pragma GCC diagnostic pop  	}  	/* DRX parameters */ @@ -1220,7 +1227,14 @@ int bssgp_tx_paging(uint16_t nsei, uint16_t ns_bvci,  	else  		bgph->pdu_type = BSSGP_PDUT_PAGING_CS;  	/* IMSI */ +/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, + * but somehow gcc (8.2) is not smart enough to figure this out and claims that + * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to + * mi[131], which is wrong */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds"  	msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2); +#pragma GCC diagnostic pop  	/* DRX Parameters */  	msgb_tvlv_put(msg, BSSGP_IE_DRX_PARAMS, 2,  			(uint8_t *) &drx_params); diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c index 77350e27..bef9bb11 100644 --- a/src/gb/gprs_bssgp_bss.c +++ b/src/gb/gprs_bssgp_bss.c @@ -183,10 +183,16 @@ int bssgp_tx_radio_status_imsi(struct bssgp_bvc_ctx *bctx, uint8_t cause,  	if (!msg)  		return -ENOMEM; - +/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11, + * but somehow gcc (8.2) is not smart enough to figure this out and claims that + * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to + * mi[131], which is wrong */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds"  	/* strip the MI type and length values (2 bytes) */  	if (imsi_len > 2)  		msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2); +#pragma GCC diagnostic pop  	LOGPC(DBSSGP, LOGL_NOTICE, "IMSI=%s ", imsi);  	return common_tx_radio_status2(msg, cause);  | 
