Change in osmo-bsc-nat[master]: bssap_conn: fix missing length check

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28582 ) Change subject: bssap_conn: fix missing length check ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc-nat/bssap_conn.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28582/comment/97cfde7b_f706dd80 PS2, Line 60: len = IP_V4_ADDR_LEN; Thanks for the suggestion! I read related libosmocore code, and now understand that all msgb code works like this, using msgb_put(msg, len) to get a pointer to a buffer to write len bytes to, and then actually write it with e.g. memcpy afterwards. msgb_put already checks that enough space is available: https://gitea.osmocom.org/osmocom/libosmocore/src/commit/0578c288ec9e33512c… So that's how the check for gsm0808_enc_aoip_trasp_addr is performed. tlv_encode_one is also using msgb functions so the length check should also work there. I think CID#273004 is a false positive now, it seems coverity can't follow that msgb_tvlv_put() would already check the length through msgb_put(msg, TVLV_GROSS_LEN(len)) correctly before writing to it. Do you agree that it's a false positive? Regarding whether to check length here: I think we should either check the length for both cases (copy IE over and replacing it) like it's done in the current version of the patch, or not check the length at all and rely on msgb_put to do it. By adding the check here we could avoid osmo_panic() if there's not enough space, but it makes the code here more complicated. Probably not worth it? -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28582 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I1fc4c81e139bab3d7d977ef9467f62d8088884db Gerrit-Change-Number: 28582 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 13 Jul 2022 09:13:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment