Skip to content

[core] Fix alignment issues in cmsg header managment (#2830)#2844

Merged
maxsharabayko merged 6 commits intoHaivision:masterfrom
yomnes0:ubsan
Jan 12, 2024
Merged

[core] Fix alignment issues in cmsg header managment (#2830)#2844
maxsharabayko merged 6 commits intoHaivision:masterfrom
yomnes0:ubsan

Conversation

@yomnes0
Copy link
Copy Markdown
Collaborator

@yomnes0 yomnes0 commented Jan 8, 2024

We used to directly cast the result of CMSG_DATA to in_pktinfo or in6_pktinfo. As stated in CMSG_FIRSTHDR manpage, alignement of the return value of this method is not guaranteed, and a memcpy should be done to manipulate it.

For some reason, the use of the mutable m_acCmsgRecvBuffer and m_acCmsgSendBuffer also caused alignemnt issues when using it as msg_control field of msghdr. Declaring a new char each time instead of using the mutable one solved this issue.

@yomnes0 yomnes0 requested a review from maxsharabayko January 8, 2024 09:45
Comment thread srtcore/channel.h Fixed
Comment thread srtcore/channel.h Fixed
Comment thread srtcore/channel.cpp Fixed
Comment thread srtcore/channel.cpp Fixed
@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Jan 9, 2024
@yomnes0 yomnes0 marked this pull request as ready for review January 11, 2024 10:46
Comment thread srtcore/channel.h Outdated
Comment thread srtcore/channel.h Outdated
Comment thread srtcore/channel.cpp Outdated
// the handshake and then addressed when sending during connection.
mh.msg_control = (m_acCmsgRecvBuffer);
mh.msg_controllen = sizeof m_acCmsgRecvBuffer;
mh.msg_control = mh_crtl_buf;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintain parentheses. This is a language-transparent marker pointing out that this variable will be used for writing.

Comment thread srtcore/channel.cpp Outdated
mh.msg_control = (m_acCmsgRecvBuffer);
mh.msg_controllen = sizeof m_acCmsgRecvBuffer;
mh.msg_control = mh_crtl_buf;
mh.msg_controllen = (sizeof(mh_crtl_buf));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses are not required here at all. Just sizeof mh_ctrl_buf.

Comment thread srtcore/channel.h Outdated
Comment thread srtcore/channel.h Outdated
Comment thread srtcore/channel.h Outdated
Copy link
Copy Markdown
Collaborator

@ethouris ethouris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed; some things below require explanation.

@maxsharabayko maxsharabayko merged commit bff0712 into Haivision:master Jan 12, 2024
@maxsharabayko maxsharabayko added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Jan 12, 2024
@maxsharabayko maxsharabayko mentioned this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants