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

Add missing zfs_refcount_destroy() in key_mapping_rele() #10246

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

gamanakis
Copy link
Contributor

@gamanakis gamanakis commented Apr 24, 2020

Motivation and Context

Testing #10228 with reference_tracking_enable=TRUE revealed a missing zfs_refcount_destroy() in key_mapping_rele().

Signed-off-by: George Amanakis [email protected]

Description

Testing #10228 with reference tracking enabled revealed a panic when running persist_l2arc_002_pos.ksh test and then unloading the zfs module. This suggested that the panic had to do with dataset encryption. Looking at the places where zfs_refcount_create is invoked, dsl_crypt.c came up. Upon looking into it the missing zfs_refcount_destroy was found.

Without the proposed fix, unloading of the zfs modules after mounting/unmounting an encrypted dataset with reference tracking enabled causes a panic:

Call Trace:
dump_stack+0x66/0x90
slab_err+0xcd/0xf2
? __kmalloc+0x174/0x260
? __kmem_cache_shutdown+0x158/0x240
__kmem_cache_shutdown.cold+0x1d/0x115
shutdown_cache+0x11/0x140
kmem_cache_destroy+0x210/0x230
spl_kmem_cache_destroy+0x122/0x3e0 [spl]
zfs_refcount_fini+0x11/0x20 [zfs]
spa_fini+0x4b/0x120 [zfs]
zfs_kmod_fini+0x6b/0xa0 [zfs]
_fini+0xa/0x68c [zfs]
__x64_sys_delete_module+0x19c/0x2b0
do_syscall_64+0x5b/0x1a0
entry_SYSCALL_64_after_hwframe+0x44/0xa9

How Has This Been Tested?

Mounting/unmounting an encrypted dataset with reference tracking enabled and the proposed fix does not lead to a panic when unloading the zfs module.

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.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Signed-off-by: George Amanakis <[email protected]>
@codecov-io
Copy link

codecov-io commented Apr 24, 2020

Codecov Report

Merging #10246 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10246      +/-   ##
==========================================
+ Coverage   79.34%   79.35%   +0.01%     
==========================================
  Files         389      389              
  Lines      123393   123394       +1     
==========================================
+ Hits        97902    97923      +21     
+ Misses      25491    25471      -20     
Flag Coverage Δ
#kernel 79.86% <100.00%> (-0.12%) ⬇️
#user 65.37% <100.00%> (+0.03%) ⬆️
Impacted Files Coverage Δ
module/zfs/dsl_crypt.c 87.52% <100.00%> (+0.01%) ⬆️
module/os/linux/spl/spl-kmem-cache.c 75.22% <0.00%> (-9.59%) ⬇️
module/lua/lmem.c 83.33% <0.00%> (-4.17%) ⬇️
module/zfs/vdev_removal.c 94.36% <0.00%> (-2.54%) ⬇️
module/zfs/vdev_indirect_mapping.c 96.61% <0.00%> (-2.42%) ⬇️
module/zfs/dsl_scan.c 83.79% <0.00%> (-1.61%) ⬇️
module/zfs/spa_log_spacemap.c 93.40% <0.00%> (-0.86%) ⬇️
module/zfs/dmu_traverse.c 96.34% <0.00%> (-0.67%) ⬇️
module/zfs/vdev_queue.c 95.71% <0.00%> (-0.62%) ⬇️
module/zfs/space_map.c 92.82% <0.00%> (-0.50%) ⬇️
... and 41 more

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 a84c92f...c6d411d. Read the comment docs.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Apr 24, 2020
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.

Thanks for running this down. Looks good, @tcaputi would you mind reviewing this as well.

Copy link
Contributor

@tcaputi tcaputi left a comment

Choose a reason for hiding this comment

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

Good catch. Thanks for finding this

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 28, 2020
@behlendorf behlendorf merged commit fa25460 into openzfs:master Apr 28, 2020
@gamanakis gamanakis mentioned this pull request Apr 28, 2020
12 tasks
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 28, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 29, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 29, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246

TEST_ZIMPORT_SKIP="yes"
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 29, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246

TEST_ZIMPORT_SKIP="yes"
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 30, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246
winny- pushed a commit to winny-/zfs that referenced this pull request May 1, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246
tonyhutter pushed a commit that referenced this pull request May 12, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes #10246
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246 
(cherry picked from commit fa25460)
as-com pushed a commit to as-com/zfs that referenced this pull request Jun 20, 2020
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reviewed-By: Brian Behlendorf <[email protected]>
Reviewed-By: Tom Caputi <[email protected]>
Signed-off-by: George Amanakis <[email protected]>
Closes openzfs#10246 
(cherry picked from commit fa25460)
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Otherwise when running with reference_tracking_enable=TRUE mounting
and unmounting an encrypted dataset panics with:

Call Trace:
 dump_stack+0x66/0x90
 slab_err+0xcd/0xf2
 ? __kmalloc+0x174/0x260
 ? __kmem_cache_shutdown+0x158/0x240
 __kmem_cache_shutdown.cold+0x1d/0x115
 shutdown_cache+0x11/0x140
 kmem_cache_destroy+0x210/0x230
 spl_kmem_cache_destroy+0x122/0x3e0 [spl]
 zfs_refcount_fini+0x11/0x20 [zfs]
 spa_fini+0x4b/0x120 [zfs]
 zfs_kmod_fini+0x6b/0xa0 [zfs]
 _fini+0xa/0x68c [zfs]
 __x64_sys_delete_module+0x19c/0x2b0
 do_syscall_64+0x5b/0x1a0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

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

4 participants