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 coverity defects: CID 186143 #8788

Merged
merged 1 commit into from
May 24, 2019

Conversation

loli10K
Copy link
Contributor

@loli10K loli10K commented May 21, 2019

Motivation and Context

Fix CID 186143: Memory - illegal accesses (USE_AFTER_FREE)

Description

This patch fixes an use-after-free in spa_import_progress_destroy() moving the kmem_free() call at the end of the function.

How Has This Been Tested?

Tested on local system with KASAN, without this patch (on rmmod zfs):

[  261.974585] ==================================================================
[  261.975839] BUG: KASAN: use-after-free in procfs_list_destroy+0x1c/0x110 [spl]
[  261.977042] Read of size 8 at addr ffff8880cc636950 by task rmmod/8141

[  261.978387] CPU: 1 PID: 8141 Comm: rmmod Tainted: P           OE     4.20.17 #3
[  261.978389] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  261.978390] Call Trace:
[  261.978396]  dump_stack+0x6f/0xa3
[  261.978400]  print_address_description+0x6b/0x270
[  261.978403]  kasan_report+0x231/0x370
[  261.978415]  ? procfs_list_destroy+0x1c/0x110 [spl]
[  261.978427]  procfs_list_destroy+0x1c/0x110 [spl]
[  261.978596]  spa_fini+0x1a7/0x2a0 [zfs]
[  261.978733]  _fini+0x88/0x2d2 [zfs]
[  261.978737]  __x64_sys_delete_module+0x1fc/0x360
[  261.978741]  ? exit_to_usermode_loop+0x63/0x120
[  261.978743]  do_syscall_64+0x6e/0x330
[  261.978746]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  261.978749] RIP: 0033:0x7f85d9722b17
[  261.978752] Code: 73 01 c3 48 8b 0d 71 c3 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 41 c3 2b 00 f7 d8 64 89 01 48
[  261.978754] RSP: 002b:00007ffc9b93edc8 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0
[  261.978756] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f85d9722b17
[  261.978758] RDX: 00007f85d9788c60 RSI: 0000000000000800 RDI: 000055abb8919220
[  261.978759] RBP: 000055abb89191c0 R08: 00007f85d99dff20 R09: 00007ffc9b93dd41
[  261.978761] R10: 00007ffc9b93eb90 R11: 0000000000000202 R12: 00007ffc9b93eff0
[  261.978762] R13: 00007ffc9b940eee R14: 0000000000000000 R15: 000055abb89191c0

[  261.978995] Allocated by task 8130:
[  261.979545]  kasan_kmalloc+0x8a/0xb0
[  261.979548]  __kmalloc_node+0x4f/0x60
[  261.979559]  spl_kmem_zalloc+0xed/0x190 [spl]
[  261.979688]  spa_init+0x17c/0x1e0 [zfs]
[  261.979689]  0xffffffffc0e60048
[  261.979691]  do_one_initcall+0x4c/0x225
[  261.979693]  do_init_module+0xe4/0x32a
[  261.979695]  load_module+0x2e24/0x4670
[  261.979697]  __do_sys_finit_module+0x94/0xe0
[  261.979699]  do_syscall_64+0x6e/0x330
[  261.979701]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

[  261.979974] Freed by task 8141:
[  261.980510]  __kasan_slab_free+0xfb/0x140
[  261.980512]  kfree+0x7b/0x100
[  261.980640]  spa_fini+0x19d/0x2a0 [zfs]
[  261.980793]  _fini+0x88/0x2d2 [zfs]
[  261.980796]  __x64_sys_delete_module+0x1fc/0x360
[  261.980798]  do_syscall_64+0x6e/0x330
[  261.980800]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

[  261.981094] The buggy address belongs to the object at ffff8880cc636900
 which belongs to the cache kmalloc-1k of size 1024
[  261.983223] The buggy address is located 80 bytes inside of
 1024-byte region [ffff8880cc636900, ffff8880cc636d00)
[  261.985228] The buggy address belongs to the page:
[  261.986016] page:ffffea0003318d80 count:1 mapcount:0 mapping:ffff88811bf8e900 index:0x0 compound_mapcount: 0
[  262.035738] flags: 0xffffc000010200(slab|head)
[  262.036547] raw: 00ffffc000010200 ffffea0003530b08 ffffea0003435908 ffff88811bf8e900
[  262.038263] raw: 0000000000000000 ffff8880cc636000 0000000100000007 0000000000000000
[  262.040162] page dumped because: kasan: bad access detected

[  262.041834] Memory state around the buggy address:
[  262.042986]  ffff8880cc636800: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
[  262.044685]  ffff8880cc636880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  262.046429] >ffff8880cc636900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  262.047957]                                                  ^
[  262.048960]  ffff8880cc636980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  262.050154]  ffff8880cc636a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  262.051371] ==================================================================
[  262.053457] ZFS: Unloaded module v0.8.0-rc5 (DEBUG mode)

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:

CID 186143: Memory - illegal accesses (USE_AFTER_FREE)

This patch fixes an use-after-free in spa_import_progress_destroy()
moving the kmem_free() call at the end of the function.

Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
@loli10K loli10K added the Status: Code Review Needed Ready for review and testing label May 21, 2019
@codecov
Copy link

codecov bot commented May 22, 2019

Codecov Report

Merging #8788 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8788      +/-   ##
==========================================
+ Coverage   78.63%   78.78%   +0.14%     
==========================================
  Files         381      381              
  Lines      117798   117797       -1     
==========================================
+ Hits        92636    92809     +173     
+ Misses      25162    24988     -174
Flag Coverage Δ
#kernel 79.36% <100%> (+0.02%) ⬆️
#user 67.35% <100%> (+0.4%) ⬆️

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 9dc41a7...8f7c739. Read the comment docs.

@chrisrd
Copy link
Contributor

chrisrd commented May 22, 2019

LGTM

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 24, 2019
@behlendorf behlendorf merged commit 75c09c5 into openzfs:master May 24, 2019
behlendorf pushed a commit that referenced this pull request Jun 7, 2019
CID 186143: Memory - illegal accesses (USE_AFTER_FREE)

This patch fixes an use-after-free in spa_import_progress_destroy()
moving the kmem_free() call at the end of the function.

Reviewed-by: Chris Dunlop <chris@onthe.net.au>
Reviewed-by: Giuseppe Di Natale <guss80@gmail.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #8788
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 7, 2019
CID 186143: Memory - illegal accesses (USE_AFTER_FREE)

This patch fixes an use-after-free in spa_import_progress_destroy()
moving the kmem_free() call at the end of the function.

Reviewed-by: Chris Dunlop <chris@onthe.net.au>
Reviewed-by: Giuseppe Di Natale <guss80@gmail.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes openzfs#8788
allanjude pushed a commit to allanjude/zfs that referenced this pull request Jun 15, 2019
CID 186143: Memory - illegal accesses (USE_AFTER_FREE)

This patch fixes an use-after-free in spa_import_progress_destroy()
moving the kmem_free() call at the end of the function.

Reviewed-by: Chris Dunlop <chris@onthe.net.au>
Reviewed-by: Giuseppe Di Natale <guss80@gmail.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes openzfs#8788
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.

6 participants