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

Questionable address adjustment in SB buffers may break alignment requirements #1020

Closed
skliper opened this issue Nov 16, 2020 · 1 comment · Fixed by #1154 or #1196
Closed

Questionable address adjustment in SB buffers may break alignment requirements #1020

skliper opened this issue Nov 16, 2020 · 1 comment · Fixed by #1154 or #1196
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Nov 16, 2020

Describe the bug
Software bus message buffers should handle most restrictive alignment requirements for a message. Address arithmetic used in manipulating buffers may break this alignment on some systems. Even if not broken, a maintenance issue since an update to CFE_SB_BufferD_t could break things unexpectedly.

/* first set ptr to actual msg buffer the same as ptr to descriptor */
address = (cpuaddr)bd;
/* increment actual msg buffer ptr beyond the descriptor */
address += sizeof(CFE_SB_BufferD_t);

/* first set ptr to actual msg buffer the same as ptr to descriptor */
address = (uint8 *)bd;
/* increment actual msg buffer ptr beyond the descriptor */
address += sizeof(CFE_SB_BufferD_t);

CFE_SB_BufferD_t *bd = (CFE_SB_BufferD_t *)((cpuaddr)Address - sizeof(CFE_SB_BufferD_t));

To Reproduce
Not confirmed, but likely won't meet alignment requirement for a message with long double.

Expected behavior
Safer to use the real buffer type (instead of void *) in the descriptor along with offsetof to size the buffer correctly (CFE_SB_Msg_t for now, or maybe CFE_SB_Buffer_t from #1019)

Code snips
See above

System observed on:
NA - inspection

Additional context
#1019, #1009

Reporter Info
Jacob Hageman - NASA/GSFC

@skliper skliper added the bug label Nov 16, 2020
@skliper
Copy link
Contributor Author

skliper commented Nov 17, 2020

Cleaning this up could also simplify the ZeroCopy logic for passing around the buffer descriptor, and it's relation to the zero copy descriptor (could eliminate CFE_SB_GetBufferFromCaller).

@skliper skliper added this to the 7.0.0 milestone Jan 21, 2021
jphickey added a commit to jphickey/cFE that referenced this issue Feb 4, 2021
Combine the "zero copy" and the normal CFE buffer descriptor into a single
unified CFE_SB_BufferD_t object.  This cleans up a bunch of extra logic
related to zero-copy buffers, including the extra descriptor object.  The
result is a simpler zero-copy design that is much less different from the
standard (non-zero-copy) message path.

All message descriptor objects are now tracked in a list by SB, not just
the zero-copy descriptors (for consistency - if any buffers need to be
tracked, they should all be tracked).
jphickey added a commit to jphickey/cFE that referenced this issue Feb 26, 2021
Combine the "zero copy" and the normal CFE buffer descriptor into a single
unified CFE_SB_BufferD_t object.  This cleans up a bunch of extra logic
related to zero-copy buffers, including the extra descriptor object.  The
result is a simpler zero-copy design that is much less different from the
standard (non-zero-copy) message path.

All message descriptor objects are now tracked in a list by SB, not just
the zero-copy descriptors (for consistency - if any buffers need to be
tracked, they should all be tracked).
jphickey added a commit to jphickey/cFE that referenced this issue Feb 26, 2021
Combine the "zero copy" and the normal CFE buffer descriptor into a single
unified CFE_SB_BufferD_t object.  This cleans up a bunch of extra logic
related to zero-copy buffers, including the extra descriptor object.  The
result is a simpler zero-copy design that is much less different from the
standard (non-zero-copy) message path.

All message descriptor objects are now tracked in a list by SB, not just
the zero-copy descriptors (for consistency - if any buffers need to be
tracked, they should all be tracked).
astrogeco added a commit that referenced this issue Mar 1, 2021
Fix #1020, refactor SB buffer descriptor object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants