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

openzfs: Account for padding in itx_t for purecap #2100

Merged
merged 1 commit into from
May 14, 2024
Merged

Conversation

jrtc27
Copy link
Member

@jrtc27 jrtc27 commented May 9, 2024

The current struct layout for purecap means there is 8 bytes of padding
at the end of itx_t after the lr_t itx_lr member, and so if olrsize is
exactly sizeof(lr_t) we underallocate by this amount, risking bounds
faults but also the assertions in zil_itx_clone and zil_itx_destroy that
itx_size is at least sizeof(itx_t). Thus, round up lrsize to ensure it
at least covers any padding. It may also be valid to leave lrsize, i.e.
itx_lr.lrc_reclen, as the unpadded value but pad itx_size, however more
assertions would need to be changed for that and it's unclear if any of
the code actually relies on the correspondence between them. Since the
code already rounds up the requested size to a multiple of 8 it's likely
safer to just round it up some more.

The current struct layout for purecap means there is 8 bytes of padding
at the end of itx_t after the lr_t itx_lr member, and so if olrsize is
exactly sizeof(lr_t) we underallocate by this amount, risking bounds
faults but also the assertions in zil_itx_clone and zil_itx_destroy that
itx_size is at least sizeof(itx_t). Thus, round up lrsize to ensure it
at least covers any padding. It may also be valid to leave lrsize, i.e.
itx_lr.lrc_reclen, as the unpadded value but pad itx_size, however more
assertions would need to be changed for that and it's unclear if any of
the code actually relies on the correspondence between them. Since the
code already rounds up the requested size to a multiple of 8 it's likely
safer to just round it up some more.
Copy link
Collaborator

@bsdjhb bsdjhb left a comment

Choose a reason for hiding this comment

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

We should also merge this commit onto the 'cheri-purecap' branch in the ZFS repo so it doesn't get lost in future ZFS merges.

@jrtc27
Copy link
Member Author

jrtc27 commented May 9, 2024

We should also merge this commit onto the 'cheri-purecap' branch in the ZFS repo so it doesn't get lost in future ZFS merges.

Yes; I can do that once this passes CI and lands

@jrtc27
Copy link
Member Author

jrtc27 commented May 14, 2024

Confirmed to work by Elise

@jrtc27 jrtc27 merged commit 5ac7eb0 into dev May 14, 2024
27 checks passed
@jrtc27 jrtc27 deleted the zil-itx-padding branch May 14, 2024 17:52
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