Thadeu Lima de Souza Cascardo
|
b51a5d0934
|
hfsplus: don't query the device logical block size multiple times
[ Upstream commit 1c82587cb57687de3f18ab4b98a8850c789bedcf ]
Devices block sizes may change. One of these cases is a loop device by
using ioctl LOOP_SET_BLOCK_SIZE.
While this may cause other issues like IO being rejected, in the case of
hfsplus, it will allocate a block by using that size and potentially write
out-of-bounds when hfsplus_read_wrapper calls hfsplus_submit_bio and the
latter function reads a different io_size.
Using a new min_io_size initally set to sb_min_blocksize works for the
purposes of the original fix, since it will be set to the max between
HFSPLUS_SECTOR_SIZE and the first seen logical block size. We still use the
max between HFSPLUS_SECTOR_SIZE and min_io_size in case the latter is not
initialized.
Tested by mounting an hfsplus filesystem with loop block sizes 512, 1024
and 4096.
The produced KASAN report before the fix looks like this:
[ 419.944641] ==================================================================
[ 419.945655] BUG: KASAN: slab-use-after-free in hfsplus_read_wrapper+0x659/0xa0a
[ 419.946703] Read of size 2 at addr ffff88800721fc00 by task repro/10678
[ 419.947612]
[ 419.947846] CPU: 0 UID: 0 PID: 10678 Comm: repro Not tainted 6.12.0-rc5-00008-gdf56e0f2f3ca #84
[ 419.949007] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
[ 419.950035] Call Trace:
[ 419.950384] <TASK>
[ 419.950676] dump_stack_lvl+0x57/0x78
[ 419.951212] ? hfsplus_read_wrapper+0x659/0xa0a
[ 419.951830] print_report+0x14c/0x49e
[ 419.952361] ? __virt_addr_valid+0x267/0x278
[ 419.952979] ? kmem_cache_debug_flags+0xc/0x1d
[ 419.953561] ? hfsplus_read_wrapper+0x659/0xa0a
[ 419.954231] kasan_report+0x89/0xb0
[ 419.954748] ? hfsplus_read_wrapper+0x659/0xa0a
[ 419.955367] hfsplus_read_wrapper+0x659/0xa0a
[ 419.955948] ? __pfx_hfsplus_read_wrapper+0x10/0x10
[ 419.956618] ? do_raw_spin_unlock+0x59/0x1a9
[ 419.957214] ? _raw_spin_unlock+0x1a/0x2e
[ 419.957772] hfsplus_fill_super+0x348/0x1590
[ 419.958355] ? hlock_class+0x4c/0x109
[ 419.958867] ? __pfx_hfsplus_fill_super+0x10/0x10
[ 419.959499] ? __pfx_string+0x10/0x10
[ 419.960006] ? lock_acquire+0x3e2/0x454
[ 419.960532] ? bdev_name.constprop.0+0xce/0x243
[ 419.961129] ? __pfx_bdev_name.constprop.0+0x10/0x10
[ 419.961799] ? pointer+0x3f0/0x62f
[ 419.962277] ? __pfx_pointer+0x10/0x10
[ 419.962761] ? vsnprintf+0x6c4/0xfba
[ 419.963178] ? __pfx_vsnprintf+0x10/0x10
[ 419.963621] ? setup_bdev_super+0x376/0x3b3
[ 419.964029] ? snprintf+0x9d/0xd2
[ 419.964344] ? __pfx_snprintf+0x10/0x10
[ 419.964675] ? lock_acquired+0x45c/0x5e9
[ 419.965016] ? set_blocksize+0x139/0x1c1
[ 419.965381] ? sb_set_blocksize+0x6d/0xae
[ 419.965742] ? __pfx_hfsplus_fill_super+0x10/0x10
[ 419.966179] mount_bdev+0x12f/0x1bf
[ 419.966512] ? __pfx_mount_bdev+0x10/0x10
[ 419.966886] ? vfs_parse_fs_string+0xce/0x111
[ 419.967293] ? __pfx_vfs_parse_fs_string+0x10/0x10
[ 419.967702] ? __pfx_hfsplus_mount+0x10/0x10
[ 419.968073] legacy_get_tree+0x104/0x178
[ 419.968414] vfs_get_tree+0x86/0x296
[ 419.968751] path_mount+0xba3/0xd0b
[ 419.969157] ? __pfx_path_mount+0x10/0x10
[ 419.969594] ? kmem_cache_free+0x1e2/0x260
[ 419.970311] do_mount+0x99/0xe0
[ 419.970630] ? __pfx_do_mount+0x10/0x10
[ 419.971008] __do_sys_mount+0x199/0x1c9
[ 419.971397] do_syscall_64+0xd0/0x135
[ 419.971761] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 419.972233] RIP: 0033:0x7c3cb812972e
[ 419.972564] Code: 48 8b 0d f5 46 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c2 46 0d 00 f7 d8 64 89 01 48
[ 419.974371] RSP: 002b:00007ffe30632548 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
[ 419.975048] RAX: ffffffffffffffda RBX: 00007ffe306328d8 RCX: 00007c3cb812972e
[ 419.975701] RDX: 0000000020000000 RSI: 0000000020000c80 RDI: 00007ffe306325d0
[ 419.976363] RBP: 00007ffe30632720 R08: 00007ffe30632610 R09: 0000000000000000
[ 419.977034] R10: 0000000000200008 R11: 0000000000000286 R12: 0000000000000000
[ 419.977713] R13: 00007ffe306328e8 R14: 00005a0eb298bc68 R15: 00007c3cb8356000
[ 419.978375] </TASK>
[ 419.978589]
Fixes: 6596528e391a ("hfsplus: ensure bio requests are not smaller than the hardware sectors")
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Link: https://lore.kernel.org/r/20241107114109.839253-1-cascardo@igalia.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
2024-12-17 13:23:59 +01:00 |
|
Chao Yu
|
8ac36e7ed3
|
hfsplus: fix to avoid false alarm of circular locking
[ Upstream commit be4edd1642ee205ed7bbf66edc0453b1be1fb8d7 ]
Syzbot report potential ABBA deadlock as below:
loop0: detected capacity change from 0 to 1024
======================================================
WARNING: possible circular locking dependency detected
6.9.0-syzkaller-10323-g8f6a15f095a6 #0 Not tainted
------------------------------------------------------
syz-executor171/5344 is trying to acquire lock:
ffff88807cb980b0 (&tree->tree_lock){+.+.}-{3:3}, at: hfsplus_file_truncate+0x811/0xb50 fs/hfsplus/extents.c:595
but task is already holding lock:
ffff88807a930108 (&HFSPLUS_I(inode)->extents_lock){+.+.}-{3:3}, at: hfsplus_file_truncate+0x2da/0xb50 fs/hfsplus/extents.c:576
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&HFSPLUS_I(inode)->extents_lock){+.+.}-{3:3}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
hfsplus_file_extend+0x21b/0x1b70 fs/hfsplus/extents.c:457
hfsplus_bmap_reserve+0x105/0x4e0 fs/hfsplus/btree.c:358
hfsplus_rename_cat+0x1d0/0x1050 fs/hfsplus/catalog.c:456
hfsplus_rename+0x12e/0x1c0 fs/hfsplus/dir.c:552
vfs_rename+0xbdb/0xf00 fs/namei.c:4887
do_renameat2+0xd94/0x13f0 fs/namei.c:5044
__do_sys_rename fs/namei.c:5091 [inline]
__se_sys_rename fs/namei.c:5089 [inline]
__x64_sys_rename+0x86/0xa0 fs/namei.c:5089
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf5/0x240 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
-> #0 (&tree->tree_lock){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x18cb/0x58e0 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
hfsplus_file_truncate+0x811/0xb50 fs/hfsplus/extents.c:595
hfsplus_setattr+0x1ce/0x280 fs/hfsplus/inode.c:265
notify_change+0xb9d/0xe70 fs/attr.c:497
do_truncate+0x220/0x310 fs/open.c:65
handle_truncate fs/namei.c:3308 [inline]
do_open fs/namei.c:3654 [inline]
path_openat+0x2a3d/0x3280 fs/namei.c:3807
do_filp_open+0x235/0x490 fs/namei.c:3834
do_sys_openat2+0x13e/0x1d0 fs/open.c:1406
do_sys_open fs/open.c:1421 [inline]
__do_sys_creat fs/open.c:1497 [inline]
__se_sys_creat fs/open.c:1491 [inline]
__x64_sys_creat+0x123/0x170 fs/open.c:1491
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf5/0x240 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&HFSPLUS_I(inode)->extents_lock);
lock(&tree->tree_lock);
lock(&HFSPLUS_I(inode)->extents_lock);
lock(&tree->tree_lock);
This is a false alarm as tree_lock mutex are different, one is
from sbi->cat_tree, and another is from sbi->ext_tree:
Thread A Thread B
- hfsplus_rename
- hfsplus_rename_cat
- hfs_find_init
- mutext_lock(cat_tree->tree_lock)
- hfsplus_setattr
- hfsplus_file_truncate
- mutex_lock(hip->extents_lock)
- hfs_find_init
- mutext_lock(ext_tree->tree_lock)
- hfs_bmap_reserve
- hfsplus_file_extend
- mutex_lock(hip->extents_lock)
So, let's call mutex_lock_nested for tree_lock mutex lock, and pass
correct lock class for it.
Fixes: 31651c607151 ("hfsplus: avoid deadlock on file truncation")
Reported-by: syzbot+6030b3b1b9bf70e538c4@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-fsdevel/000000000000e37a4005ef129563@google.com
Cc: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
Signed-off-by: Chao Yu <chao@kernel.org>
Link: https://lore.kernel.org/r/20240607142304.455441-1-chao@kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
2024-11-23 23:19:56 +01:00 |
|