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

Unaligned access in random_bytes() and rand_get_bytes() #195

Closed
rw8896 opened this issue Oct 12, 2021 · 4 comments · Fixed by #1166
Closed

Unaligned access in random_bytes() and rand_get_bytes() #195

rw8896 opened this issue Oct 12, 2021 · 4 comments · Fixed by #1166
Labels
enhancement New feature or request

Comments

@rw8896
Copy link
Contributor

rw8896 commented Oct 12, 2021

*((uint64 *)output) = temp_rand;

*((uint64 *)RandBuffer) = temp_rand;

These assignments may cause unaligned access when output or RandBuffer is not at alignment of uint64_t.
Better to use copy_mem() instead.

@rw8896 rw8896 changed the title Unaligned access Unaligned access in random_bytes() and rand_get_bytes() Oct 12, 2021
@steven-bellock
Copy link
Contributor

Part of the issue is the SPDM specification itself, and how fields are laid out within a request or response. There's also endianness issues as well. In the short term we can fix this; in the long term need a strategy on whether to accommodate all alignment issues.

@steven-bellock steven-bellock added the enhancement New feature or request label Oct 18, 2021
@steven-bellock
Copy link
Contributor

Will defer to STABLE 0.2.

@jyao1
Copy link
Member

jyao1 commented Jan 31, 2022

@rw8896, do you want to propose a fix?

@rw8896
Copy link
Contributor Author

rw8896 commented Jan 31, 2022

As @steven-bellock commented, the short term solution is just to avoid the uint64 assignment in these two rand.c files.
We could simply replace it with copy_mem().

I am not sure about the long term strategy to address the alignment and endianness issues though.

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

Successfully merging a pull request may close this issue.

3 participants