lkml.org 
[lkml]   [2022]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH -next] ext4: fix bug_on in ext4_iomap_begin as race between bmap and write
From
Date


On 2022/6/15 23:21, Ritesh Harjani wrote:
> On 22/06/15 09:58PM, Ye Bin wrote:
>> We got issue as follows:
>> ------------[ cut here ]------------
>> WARNING: CPU: 3 PID: 9310 at fs/ext4/inode.c:3441 ext4_iomap_begin+0x182/0x5d0
>> RIP: 0010:ext4_iomap_begin+0x182/0x5d0
>> RSP: 0018:ffff88812460fa08 EFLAGS: 00010293
>> RAX: ffff88811f168000 RBX: 0000000000000000 RCX: ffffffff97793c12
>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
>> RBP: ffff88812c669160 R08: ffff88811f168000 R09: ffffed10258cd20f
>> R10: ffff88812c669077 R11: ffffed10258cd20e R12: 0000000000000001
>> R13: 00000000000000a4 R14: 000000000000000c R15: ffff88812c6691ee
>> FS: 00007fd0d6ff3740(0000) GS:ffff8883af180000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007fd0d6dda290 CR3: 0000000104a62000 CR4: 00000000000006e0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>> iomap_apply+0x119/0x570
>> iomap_bmap+0x124/0x150
>> ext4_bmap+0x14f/0x250
>> bmap+0x55/0x80
>> do_vfs_ioctl+0x952/0xbd0
>> __x64_sys_ioctl+0xc6/0x170
>> do_syscall_64+0x33/0x40
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Above issue may happen as follows:
>> bmap write
>> bmap
>> ext4_bmap
>> iomap_bmap
>> ext4_iomap_begin
>> ext4_file_write_iter
>> ext4_buffered_write_iter
>> generic_perform_write
>> ext4_da_write_begin
>> ext4_da_write_inline_data_begin
>> ext4_prepare_inline_data
>> ext4_create_inline_data
>> ext4_set_inode_flag(inode,
>> EXT4_INODE_INLINE_DATA);
>> if (WARN_ON_ONCE(ext4_has_inline_data(inode))) ->trigger bug_on
>>
>> To solved above issue hold inode lock in ext4_bamp.
> ^^^ ext4_bmap()
>
> I checked the paths where bmap() kernel api can be called i.e. from jbd2/fc and
> generic_swapfile_activate() (apart from ioctl())
> For jbd2, it will be called with j_inode within bmap(), hence taking a inode lock
> of the inode passed within ext4_bmap() (j_inode in this case) should be safe here.
> Same goes with swapfile path as well.
>
> However I feel maybe we should hold inode_lock_shared() since there is no
> block/extent map layout changes that can happen via ext4_bmap().
> Hence read lock is what IMO should be used here.
>
> -ritesh
Thank you for your advice.
>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>> fs/ext4/inode.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 53877ffe3c41..f4a95c80f644 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -3142,13 +3142,15 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
>> {
>> struct inode *inode = mapping->host;
>> journal_t *journal;
>> + sector_t ret = 0;
>> int err;
>>
>> + inode_lock(inode);
>> /*
>> * We can get here for an inline file via the FIBMAP ioctl
>> */
>> if (ext4_has_inline_data(inode))
>> - return 0;
>> + goto out;
>>
>> if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) &&
>> test_opt(inode->i_sb, DELALLOC)) {
>> @@ -3187,10 +3189,14 @@ static sector_t ext4_bmap(struct address_space *mapping, sector_t block)
>> jbd2_journal_unlock_updates(journal);
>>
>> if (err)
>> - return 0;
>> + goto out;
>> }
>>
>> - return iomap_bmap(mapping, block, &ext4_iomap_ops);
>> + ret = iomap_bmap(mapping, block, &ext4_iomap_ops);
>> +
>> +out:
>> + inode_unlock(inode);
>> + return ret;
>> }
>>
>> static int ext4_read_folio(struct file *file, struct folio *folio)
>> --
>> 2.31.1
>>
> .
>

\
 
 \ /
  Last update: 2022-06-16 03:47    [W:1.502 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site