osmocom
/
libosmocore
mirror of https://gerrit.osmocom.org/libosmocore
11
0
Fork 0
Code Issues Releases Wiki Activity

gsm/{bsslap,bssmap_le}: zero-initialize structs using memset() 

In the unit tests we're using memcmp() to compare decoding results
against the expected results.  This is a reasonable approach, but
there is a pitfall: not only the struct fields are compared, but
also the padding bytes preceding/following them.

When using gcc's extension zero-initializer {} or even the standard
approved { 0 } zero-initializer, padding bytes are not guaranteed
to be zeroed.  Even worse, according to [1], the init behavior is
inconsistent between gcc and clang and optimization levels.

All decoding functions in {bsslap,bssmap_le}.c currently use gcc's
extension zero-initializer {}.  This is not a problem when building
with CC=gcc, but with CC=clang the bssmap_le_test fails due to
mismatch of padding bytes in struct lcs_cause_ie:

  [4] PERFORM LOCATION RESPONSE: ERROR: decoded PDU != encoded PDU
  [5] PERFORM LOCATION RESPONSE: ERROR: decoded PDU != encoded PDU
  [6] PERFORM LOCATION ABORT: ERROR: decoded PDU != encoded PDU

Out of the known struct initialization methods, only the memset()
has consistent behavior and sets all bytes to zero, including the
padding ones.  Using it fixes the bssmap_le_test for CC=clang.

[1] https://interrupt.memfault.com/blog/c-struct-padding-initialization

Change-Id: Ib16964b16eb04315efc416164ed46c15b5dc7254
Fixes: OS#5923
This commit is contained in:
Vadim Yanitskiy 2023-02-25 05:52:37 +07:00 committed by laforge
parent 6b69b554f7
commit 7b9b3074a2
2 changed files with 10 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/gsm/bsslap.c
View File

@ -217,7 +217,7 @@ int osmo_bsslap_dec(struct bsslap_pdu *pdu,
int ies_len;
struct tlv_parsed tp;
*pdu = (struct bsslap_pdu){};
memset(pdu, 0x00, sizeof(*pdu));
if (err)
*err = NULL;

17
src/gsm/bssmap_le.c
View File

@ -180,7 +180,7 @@ int osmo_bssmap_le_ie_dec_location_type(struct bssmap_le_location_type *lt,
struct osmo_bssmap_le_err **err, void *err_ctx,
const uint8_t *elem, uint8_t len)
{
*lt = (struct bssmap_le_location_type){};
memset(lt, 0x00, sizeof(*lt));
if (!elem || len < 1)
DEC_ERR(-EINVAL, msgt, iei, LCS_CAUSE_UNSPECIFIED, "zero length");
@ -348,7 +348,7 @@ int osmo_lcs_cause_dec(struct lcs_cause_ie *lcs_cause,
struct osmo_bssmap_le_err **err, void *err_ctx,
const uint8_t *data, uint8_t len)
{
*lcs_cause = (struct lcs_cause_ie){};
memset(lcs_cause, 0x00, sizeof(*lcs_cause));
if (!data || len < 1)
DEC_ERR(-EINVAL, msgt, iei, LCS_CAUSE_UNSPECIFIED, "zero length");
@ -558,7 +558,7 @@ static int osmo_bssmap_le_dec_perform_loc_req(struct bssmap_le_perform_loc_req *
struct osmo_bssmap_le_err **err, void *err_ctx,
const struct tlv_parsed *tp)
{
*params = (struct bssmap_le_perform_loc_req){};
memset(params, 0x00, sizeof(*params));
DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_LOCATION_TYPE, osmo_bssmap_le_ie_dec_location_type,
&params->location_type);
@ -606,7 +606,7 @@ static int osmo_bssmap_le_dec_perform_loc_resp(struct bssmap_le_perform_loc_resp
struct osmo_bssmap_le_err **err, void *err_ctx,
const struct tlv_parsed *tp)
{
*params = (struct bssmap_le_perform_loc_resp){};
memset(params, 0x00, sizeof(*params));
DEC_IE_OPTIONAL_FLAG(msgt, BSSMAP_LE_IEI_GEO_LOCATION, osmo_bssmap_le_ie_dec_gad, &params->location_estimate,
params->location_estimate_present);
@ -630,7 +630,7 @@ static int osmo_bssmap_le_dec_perform_loc_abort(struct lcs_cause_ie *params,
struct osmo_bssmap_le_err **err, void *err_ctx,
const struct tlv_parsed *tp)
{
*params = (struct lcs_cause_ie){};
memset(params, 0x00, sizeof(*params));
DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_LCS_CAUSE, osmo_lcs_cause_dec, params);
return 0;
@ -647,7 +647,8 @@ static int osmo_bssmap_le_dec_conn_oriented_info(struct bssmap_le_conn_oriented_
struct osmo_bssmap_le_err **err, void *err_ctx,
const struct tlv_parsed *tp)
{
*params = (struct bssmap_le_conn_oriented_info){};
memset(params, 0x00, sizeof(*params));
DEC_IE_MANDATORY(msgt, BSSMAP_LE_IEI_APDU, osmo_bssmap_le_ie_dec_apdu, &params->apdu);
return 0;
}
@ -711,7 +712,7 @@ static int osmo_bssmap_le_dec(struct bssmap_le_pdu *pdu,
int ies_len;
struct tlv_parsed tp;
*pdu = (struct bssmap_le_pdu){};
memset(pdu, 0x00, sizeof(*pdu));
if (len < 1)
DEC_ERR(-EINVAL, -1, -1, LCS_CAUSE_UNSPECIFIED, "zero length");
@ -798,7 +799,7 @@ int osmo_bssap_le_dec(struct bssap_le_pdu *pdu, struct osmo_bssap_le_err **err,
return RC; \
} while(0)
*pdu = (struct bssap_le_pdu){};
memset(pdu, 0x00, sizeof(*pdu));
h = msgb_l2(msg);
if (!h)