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

Fix object reclaim when using large dnodes #7433

Merged
merged 1 commit into from
Apr 17, 2018

Conversation

tcaputi
Copy link
Contributor

@tcaputi tcaputi commented Apr 13, 2018

Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Fixes: #7097

Signed-off-by: Tom Caputi [email protected]

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@tcaputi
Copy link
Contributor Author

tcaputi commented Apr 13, 2018

@morphinz is testing this PR now (for his system which is exhibiting the problem) and I will be adding a test case on Monday.

@@ -676,6 +676,7 @@ dnode_reallocate(dnode_t *dn, dmu_object_type_t ot, int blocksize,
ASSERT(DMU_OT_IS_VALID(bonustype));
ASSERT3U(bonuslen, <=,
DN_BONUS_SIZE(spa_maxdnodesize(dmu_objset_spa(dn->dn_objset))));
ASSERT3U(bonuslen, <=, DN_BONUS_SIZE(dn_slots << DNODE_SHIFT));
Copy link
Contributor

Choose a reason for hiding this comment

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

With this ASSERT in place passing a dn_slots == 0 no longer makes sense. This is exactly what dmu_object_reclaim() does, it will need to be updated. The following line could also be removed since it's no longer useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. Apologies. I meant to make this a WIP until l had the chance to write tests for it. I should have that done on Monday.

@tcaputi tcaputi force-pushed the dnode_reclaim_fix branch from 9ac1457 to d61aa12 Compare April 14, 2018 05:42
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Fixes: openzfs#7097

Signed-off-by: Tom Caputi <[email protected]>
@tcaputi tcaputi force-pushed the dnode_reclaim_fix branch from d61aa12 to 27c5713 Compare April 16, 2018 20:00
@codecov
Copy link

codecov bot commented Apr 17, 2018

Codecov Report

Merging #7433 into master will decrease coverage by 0.36%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7433      +/-   ##
==========================================
- Coverage   76.53%   76.16%   -0.37%     
==========================================
  Files         335      330       -5     
  Lines      107100   104295    -2805     
==========================================
- Hits        81965    79441    -2524     
+ Misses      25135    24854     -281
Flag Coverage Δ
#kernel 75.7% <100%> (-1.35%) ⬇️
#user 65.5% <0%> (-0.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d68ac65...27c5713. Read the comment docs.

@behlendorf behlendorf merged commit e14a32b into openzfs:master Apr 17, 2018
dweeezil pushed a commit to dweeezil/zfs that referenced this pull request Aug 27, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
dweeezil pushed a commit to dweeezil/zfs that referenced this pull request Aug 28, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
dweeezil pushed a commit to dweeezil/zfs that referenced this pull request Aug 28, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 28, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 28, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 30, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 30, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 30, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 5, 2018
Currently, when the receive_object() code wants to reclaim an
object, it always assumes that the dnode is the legacy 512 bytes,
even when the incoming bonus buffer exceeds this length. This
causes a buffer overflow if --enable-debug is not provided and
triggers an ASSERT if it is. This patch resolves this issue and
adds an ASSERT to ensure this can't happen again.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes openzfs#7097
Closes openzfs#7433
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.

Kernel panics when i start incremental recv.
2 participants