<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +2</span></p><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/22833">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/22833/1/src/xua_snm.c">File src/xua_snm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/22833/1/src/xua_snm.c@313">Patch Set #1, Line 313:</a> <code style="font-family:monospace,monospace">               const uint32_t *aff_pc = (const uint32_t *)ie_aff_pc->dat;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it's not any worse than xua_msg_part_get_u32() which we use all the time in this codebase.  So We always had the assumption that those IEs are aligned at 32bit boundaries.  For sure inside XUA everything is 32bit aligned, as the tag and length values are 16bit themselves (header == 32bit) and they are always padded to 32bit.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The message header also is 32bit.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Howver, the interesting questin is whether we can make the assumption that the start of the message is aligned with the natural alignment boundary of the underlying architecture.</p><p style="white-space: pre-wrap; word-wrap: break-word;">As the M3UA header starts at the beginning of what we get returned from the read/recv/recvmsg/... call on the socket, it's actually up to us what kind of buffer we pass.</p><p style="white-space: pre-wrap; word-wrap: break-word;">As long as msgb_alloc_headroom() will result in msgb->data being 32bit aligned, we are good.  As 'struct msgb' is not __attribute__((packed), I would expect that the unsignd char _data[0] is properly aligned.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So in the end, I think we're good.</p><p style="white-space: pre-wrap; word-wrap: break-word;">One could consider adding a related check or even an assert into the library, but that's out of the scope of this change.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/22833">change 22833</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmo-sccp/+/22833"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-sccp </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ie9a45b905bc17e7b695e15fe12ba4bbadcd032bf </div>
<div style="display:none"> Gerrit-Change-Number: 22833 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 11 Feb 2021 18:04:10 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>