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

abd_get_offset_struct() may allocate new abd #11683

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

lundman
Copy link
Contributor

@lundman lundman commented Mar 3, 2021

ABD can be initialised twice - which is a panic on some platforms.

Motivation and Context

Ensure any call to abd_init_struct(); has a corresponding call to abd_fini_struct();

Description

Even when supplied with an abd to abd_get_offset_struct(), the call
to abd_get_offset_impl() can allocate a different abd. Ensure to
call abd_fini_struct() on the abd that is not used.

This was easily triggered on macOS with zpool_upgrade zfs-tests. With the stack:

 : mach_kernel : _panic + 0x54
 : _abd_get_offset_struct.cold.1 + 0x37
 : _abd_get_offset_struct + 0x8b
 : _vdev_raidz_map_alloc + 0x2e1
 : _vdev_raidz_io_start + 0x33
 : _zio_vdev_io_start + 0x21c
 : _zio_nowait + 0x20c

How Has This Been Tested?

Lots of tests on macOS - now to make sure it doesn't break for others.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Even when supplied with an abd to abd_get_offset_struct(), the call
to abd_get_offset_impl() can allocate a different abd. Ensure to
call abd_fini_struct() on the abd that is not used.

Signed-off-by: Jorgen Lundman <[email protected]>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Mar 3, 2021
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Looks good. Seems like this issue could be a problem for all the platforms to varying degrees.

Copy link
Member

@ahrens ahrens left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Mar 3, 2021
@behlendorf behlendorf merged commit 8a6d444 into openzfs:master Mar 5, 2021
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Even when supplied with an abd to abd_get_offset_struct(), the call
to abd_get_offset_impl() can allocate a different abd. Ensure to
call abd_fini_struct() on the abd that is not used.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#11683
rottegift added a commit to rottegift/openzfs that referenced this pull request May 2, 2021
rottegift added a commit to rottegift/openzfs that referenced this pull request May 3, 2021
rottegift added a commit to rottegift/openzfs that referenced this pull request May 5, 2021
rottegift added a commit to rottegift/openzfs that referenced this pull request May 6, 2021
@lundman lundman deleted the macOS_pr14 branch May 28, 2021 05:20
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Even when supplied with an abd to abd_get_offset_struct(), the call
to abd_get_offset_impl() can allocate a different abd. Ensure to
call abd_fini_struct() on the abd that is not used.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#11683
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants