Skip to content
This repository has been archived by the owner on Feb 26, 2020. It is now read-only.

BUG: KASAN: global-out-of-bounds in crgetgroups+0x31/0x60 #556

Closed
tuxoko opened this issue May 28, 2016 · 3 comments
Closed

BUG: KASAN: global-out-of-bounds in crgetgroups+0x31/0x60 #556

tuxoko opened this issue May 28, 2016 · 3 comments

Comments

@tuxoko
Copy link
Contributor

tuxoko commented May 28, 2016

I'm not familiar with this, so I'm not sure what's wrong.

[  432.864568] ==================================================================
[  432.865199] BUG: KASAN: global-out-of-bounds in crgetgroups+0x31/0x60 [spl] at addr ffffffff824c7fb0
[  432.865992] Read of size 8 by task splat/4174
[  432.866412] Address belongs to variable init_groups+0x90/0x260
[  432.866954] CPU: 0 PID: 4174 Comm: splat Tainted: G    B      OE   4.6.0+ #5
[  432.867576] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
[  432.868407]  ffffffff824c7fb0 0000000036c4c87b ffff88006576fa10 ffffffff816350b8
[  432.869107]  ffffffff824c7f20 ffff88006576faa8 ffff88006576fa98 ffffffff81313f68
[  432.869808]  0000000000000000 ffff88011ad10f90 0000000000000246 ffffffff810ea4ed
[  432.870515] Call Trace:
[  432.870747]  [<ffffffff816350b8>] dump_stack+0x63/0x8b
[  432.871251]  [<ffffffff81313f68>] kasan_report_error+0x528/0x560
[  432.871787]  [<ffffffff810ea4ed>] ? __queue_work+0x21d/0x660
[  432.872290]  [<ffffffff81314538>] kasan_report+0x58/0x60
[  432.872772]  [<ffffffff817e4c00>] ? tty_buffer_lock_exclusive+0x10/0x20
[  432.873375]  [<ffffffffc0ac6bc1>] ? crgetgroups+0x31/0x60 [spl]
[  432.873926]  [<ffffffff81312ece>] __asan_load8+0x5e/0x70
[  432.874486]  [<ffffffffc0ac6bc1>] crgetgroups+0x31/0x60 [spl]
[  432.875018]  [<ffffffffc0b1dffe>] splat_cred_test2+0x18e/0x690 [splat]
[  432.875576]  [<ffffffff817dde51>] ? n_tty_write+0x471/0x830
[  432.876054]  [<ffffffffc0b1de70>] ? splat_cred_test3+0x8d0/0x8d0 [splat]
[  432.876647]  [<ffffffff81132e12>] ? __wake_up_common+0x42/0xc0
[  432.877174]  [<ffffffff81132ed4>] ? __wake_up+0x44/0x50
[  432.877654]  [<ffffffffc0af88c9>] splat_unlocked_ioctl+0x399/0xb70 [splat]
[  432.878284]  [<ffffffffc0af8530>] ? splat_subsystem_find+0x80/0x80 [splat]
[  432.878912]  [<ffffffff813b2cc0>] ? fsnotify+0x760/0x7f0
[  432.879400]  [<ffffffff8130f28a>] ? __slab_free+0x9a/0x2d0
[  432.879901]  [<ffffffff81344900>] ? default_llseek+0x120/0x120
[  432.880435]  [<ffffffff81168e77>] ? call_rcu_sched+0x17/0x20
[  432.880954]  [<ffffffff813665fb>] do_vfs_ioctl+0x14b/0x8e0
[  432.881459]  [<ffffffff812b5e8d>] ? kzfree+0x2d/0x40
[  432.881914]  [<ffffffff813664b0>] ? ioctl_preallocate+0x160/0x160
[  432.882476]  [<ffffffff815a1f88>] ? apparmor_file_permission+0x18/0x20
[  432.883070]  [<ffffffff81346e2d>] ? vfs_write+0x18d/0x260
[  432.883563]  [<ffffffff81348edf>] ? SyS_write+0xcf/0x140
[  432.884049]  [<ffffffff81366e09>] SyS_ioctl+0x79/0x90
[  432.884514]  [<ffffffff81d0f8b6>] entry_SYSCALL_64_fastpath+0x1e/0xa8
[  432.885099] Memory state around the buggy address:
[  432.885537]  ffffffff824c7e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
[  432.886168]  ffffffff824c7f00: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
[  432.886822] >ffffffff824c7f80: 00 00 00 00 00 00 fa fa fa fa fa fa 00 00 00 00
[  432.887461]                                      ^
[  432.887889]  ffffffff824c8000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  432.888525]  ffffffff824c8080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  432.889161] ==================================================================
@tuxoko
Copy link
Contributor Author

tuxoko commented May 28, 2016

I think I know what the problem is.

crgetgroups is trying to access init_groups.blocks[0].
But init_groups.blocks is actually zero length.
http://lxr.free-electrons.com/source/include/linux/cred.h#L32
http://lxr.free-electrons.com/source/kernel/cred.c#L38

@dweeezil
Copy link
Contributor

I don't think crgetgroups() should be called if the previous call to crgetngroups() returns 0. Both test1 and test2 have the problem but test3 is ok.

@tuxoko
Copy link
Contributor Author

tuxoko commented May 30, 2016

Yes, check is necessary.
Also, the use of kcred is not safe at all. When you refer to task_struct->cred other than current, you need to rcu dereference it. Otherwise, it will change at anytime.

This was referenced Oct 19, 2016
Closed
Closed
behlendorf pushed a commit that referenced this issue Oct 20, 2016
@behlendorf
Fix splat-cred.c cred usage
87063d7 
No need to crhold current_cred(), fix possible leak in splat_cred_test2

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Chunwei Chen <[email protected]>
Closes #556
behlendorf pushed a commit to behlendorf/spl that referenced this issue Jan 20, 2017
@behlendorf
Fix crgetgroups out-of-bound and misc cred fix
0be8582 
init_groups has 0 nblocks, therefore calling the current crgetgroups with
init_groups would result in out-of-bound access. We fix this by returning NULL
when nblocks is 0.

Cap crgetngroups to NGROUPS_PER_BLOCK, since crgetgroups will only return
blocks[0].

Also, remove all get_group_info. The cred already holds reference on the
group_info, and cred is not mutable. So there's no reason to hold extra
reference, if we hold cred.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Chunwei Chen <[email protected]>
Closes openzfs#556
behlendorf pushed a commit to behlendorf/spl that referenced this issue Feb 2, 2017
@behlendorf
Fix crgetgroups out-of-bound and misc cred fix
2841ee9 
init_groups has 0 nblocks, therefore calling the current crgetgroups with
init_groups would result in out-of-bound access. We fix this by returning NULL
when nblocks is 0.

Cap crgetngroups to NGROUPS_PER_BLOCK, since crgetgroups will only return
blocks[0].

Also, remove all get_group_info. The cred already holds reference on the
group_info, and cred is not mutable. So there's no reason to hold extra
reference, if we hold cred.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Chunwei Chen <[email protected]>
Closes openzfs#556
behlendorf pushed a commit to behlendorf/spl that referenced this issue Feb 3, 2017
@behlendorf
Fix splat-cred.c cred usage
53cd26e 
No need to crhold current_cred(), fix possible leak in splat_cred_test2

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Chunwei Chen <[email protected]>
Closes openzfs#556
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Linux 4.9 compat tuxoko/spl
2 participants
@tuxoko @dweeezil