laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27590 ) Change subject: SMSCB: Preserve padding at end of page in CBSP -> RSL conversion ...................................................................... SMSCB: Preserve padding at end of page in CBSP -> RSL conversion When copying the CBS page content from CBSP to RSL data structures, we use the User Information Length as length argument in the memcpy. The logic for that is that only this part of the message contains valid data. However, as the user information length is not passed on via RSL or transmitted over the air, the receiving MS will get a page with zero-initialized padding, rather than whatever the originator of the message has specified. As zero bytes in the 8bit domain might get translated into @-characters in the 7bit domain, this creates problems. So instead, let's always copy the entire page (82 bytes) to ensure transparency when passing on information from CBSP to RSL. Change-Id: Iffcf1f6a7d41a08a2feffc6f2ac5634d940b63aa Closes: SYS#5904 --- M src/osmo-bsc/smscb.c 1 file changed, 7 insertions(+), 1 deletion(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/osmo-bsc/smscb.c b/src/osmo-bsc/smscb.c index 542453c..1640880 100644 --- a/src/osmo-bsc/smscb.c +++ b/src/osmo-bsc/smscb.c @@ -22,6 +22,7 @@ #include <limits.h> #include <osmocom/core/stats.h> +#include <osmocom/core/utils.h> #include <osmocom/core/select.h> #include <osmocom/core/msgb.h> #include <osmocom/core/talloc.h> @@ -384,6 +385,9 @@ page = &smscb->page[i++]; msg_param = (struct gsm23041_msg_param_gsm *) &page->data[0]; + /* ensure we don't overflow in the memcpy below */ + osmo_static_assert(sizeof(*page) > sizeof(*msg_param) + sizeof(cont->data), smscb_space); + /* build 6 byte header according to TS 23.041 9.4.1.2 */ osmo_store16be(wrepl->new_serial_nr, &msg_param->serial_nr); osmo_store16be(wrepl->msg_id, &msg_param->message_id); @@ -393,7 +397,9 @@ OSMO_ASSERT(cont->user_len <= ARRAY_SIZE(cont->data)); OSMO_ASSERT(cont->user_len <= ARRAY_SIZE(page->data) - sizeof(*msg_param)); - memcpy(&msg_param->content, cont->data, cont->user_len); + /* we must not use cont->user_len as length here, as it would truncate any + * possible 7-bit padding at the end. Always copy the whole page */ + memcpy(&msg_param->content, cont->data, sizeof(cont->data)); bytes_used = sizeof(*msg_param) + cont->user_len; /* compute number of valid blocks in page */ page->num_blocks = bytes_used / 22; -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27590 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Iffcf1f6a7d41a08a2feffc6f2ac5634d940b63aa Gerrit-Change-Number: 27590 Gerrit-PatchSet: 2 Gerrit-Owner: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged