Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Fix unaligned memory access #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vcgomes
Copy link

@vcgomes vcgomes commented Apr 3, 2023

This is a mix of a bug report and a RFC, the bug report is that I had some problems with packets being dropped on the receiving side because of wrong checksums, and while investigating that I found some unaligned memory access warnings, which seemed to be the cause of the wrong checksums.

The RFC part is how I tried to fix that, but it is only lightly tested this and I am not sure that this is the better way to solve the issue for the project. I tried to keep the code as similar as I could to the original, so any deviation from the original "spirit" could be detected "at a glance".

I am thinking that adding (and using) the complete "get_unaligned + endianess conversion" calls (for example, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/asm-generic/unaligned.h#n60) could be a good idea, but opinions are welcome.

Allow the user to control the value of CFLAGS without having to change
the Makefile.

Will allow easier experiments with different compilation
flags/sanitizer options.
Solve some undefined memory access errors detected by the compiler.
Changing the compilation flags to:

     CFLAGS = -O2 -Wall -g -fsanitize=undefined

We get these kinds of errors during runtime:

ender.c:150:26: runtime error: store to misaligned address 0x7ffca4a974fa for type 'uint32_t', which requires 4 byte alignment
0x7ffca4a974fa: note: pointer points here
 40 11  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
sender.c:151:26: runtime error: store to misaligned address 0x7ffca4a974fe for type 'uint32_t', which requires 4 byte alignment
0x7ffca4a974fe: note: pointer points here
 ac 12 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^
sender.c:173:27: runtime error: store to misaligned address 0x7ffca4a97522 for type 'uint64_t', which requires 8 byte alignment
0x7ffca4a97522: note: pointer points here
 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
sender.c:174:27: runtime error: store to misaligned address 0x7ffca4a9752a for type 'uint64_t', which requires 8 byte alignment
0x7ffca4a9752a: note: pointer points here
 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
sender.c:175:27: runtime error: store to misaligned address 0x7ffca4a97532 for type 'uint64_t', which requires 8 byte alignment
0x7ffca4a97532: note: pointer points here
 36 00  dd 17 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^
@mlichvar
Copy link
Owner

mlichvar commented Apr 4, 2023

CFLAGS can be set on the make command line, e.g. make CFLAGS="-O3 -g".

For the misaligned access, I'd suggest to allocate a buffer on stack and memcpy the frame there with an offset of 6 bytes, which should make all the fields aligned. Similarly on the sender side, make the buffers larger by 6 bytes and adjust the data pointer. This should be simpler and maybe also faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants