* [PATCH 0/2] fix race between setxattr and write back @ 2023-08-11 3:38 Ye Bin 2023-08-11 3:38 ` [PATCH 1/2] ext2: " Ye Bin 2023-08-11 3:38 ` [PATCH 2/2] ext2: dump current reservation window info before BUG Ye Bin 0 siblings, 2 replies; 6+ messages in thread From: Ye Bin @ 2023-08-11 3:38 UTC (permalink / raw) To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin Ye Bin (2): ext2: fix race between setxattr and write back ext2: dump current reservation window info before BUG fs/ext2/balloc.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] ext2: fix race between setxattr and write back 2023-08-11 3:38 [PATCH 0/2] fix race between setxattr and write back Ye Bin @ 2023-08-11 3:38 ` Ye Bin 2023-08-14 12:46 ` Jan Kara 2023-08-11 3:38 ` [PATCH 2/2] ext2: dump current reservation window info before BUG Ye Bin 1 sibling, 1 reply; 6+ messages in thread From: Ye Bin @ 2023-08-11 3:38 UTC (permalink / raw) To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin There's a issue as follows: Block Allocation Reservation Windows Map (ext2_try_to_allocate_with_rsv): reservation window 0x000000006f105382 start: 0, end: 0 reservation window 0x000000008fd1a555 start: 1044, end: 1059 Window map complete. kernel BUG at fs/ext2/balloc.c:1158! invalid opcode: 0000 [#1] PREEMPT SMP KASAN RIP: 0010:ext2_try_to_allocate_with_rsv.isra.0+0x15c4/0x1800 Call Trace: <TASK> ext2_new_blocks+0x935/0x1690 ext2_new_block+0x73/0xa0 ext2_xattr_set2+0x74f/0x1730 ext2_xattr_set+0x12b6/0x2260 ext2_xattr_user_set+0x9c/0x110 __vfs_setxattr+0x139/0x1d0 __vfs_setxattr_noperm+0xfc/0x370 __vfs_setxattr_locked+0x205/0x2c0 vfs_setxattr+0x19d/0x3b0 do_setxattr+0xff/0x220 setxattr+0x123/0x150 path_setxattr+0x193/0x1e0 __x64_sys_setxattr+0xc8/0x170 do_syscall_64+0x35/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd Above issue may happens as follows: setxattr write back ext2_xattr_set ext2_xattr_set2 ext2_new_block ext2_new_blocks ext2_try_to_allocate_with_rsv alloc_new_reservation --> group=0 [0, 1023] rsv [1016, 1023] do_writepages mpage_writepages write_cache_pages __mpage_writepage ext2_get_block ext2_get_blocks ext2_alloc_branch ext2_new_blocks ext2_try_to_allocate_with_rsv alloc_new_reservation -->group=1 [1024, 2047] rsv [1044, 1059] if ((my_rsv->rsv_start > group_last_block) || (my_rsv->rsv_end < group_first_block) rsv_window_dump BUG(); Now ext2 mkwrite delay allocate new blocks. So there maybe allocate blocks when do write back. However, there is no concurrent protection between ext2_xattr_set() and do_writepages(). To solve about issue hold '&ei->truncate_mutex' lock when new block for xattr. Signed-off-by: Ye Bin <yebin10@huawei.com> --- fs/ext2/balloc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index c8049c90323d..039f655febfd 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -1432,8 +1432,14 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal, ext2_fsblk_t ext2_new_block(struct inode *inode, unsigned long goal, int *errp) { unsigned long count = 1; + struct ext2_inode_info *ei = EXT2_I(inode); + ext2_fsblk_t ret; + + mutex_lock(&ei->truncate_mutex); + ret = ext2_new_blocks(inode, goal, &count, errp); + mutex_unlock(&ei->truncate_mutex); - return ext2_new_blocks(inode, goal, &count, errp); + return ret; } #ifdef EXT2FS_DEBUG -- 2.31.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ext2: fix race between setxattr and write back 2023-08-11 3:38 ` [PATCH 1/2] ext2: " Ye Bin @ 2023-08-14 12:46 ` Jan Kara 2023-08-14 15:42 ` yebin (H) 0 siblings, 1 reply; 6+ messages in thread From: Jan Kara @ 2023-08-14 12:46 UTC (permalink / raw) To: Ye Bin; +Cc: jack, linux-ext4, linux-kernel On Fri 11-08-23 11:38:56, Ye Bin wrote: > There's a issue as follows: > Block Allocation Reservation Windows Map (ext2_try_to_allocate_with_rsv): > reservation window 0x000000006f105382 start: 0, end: 0 > reservation window 0x000000008fd1a555 start: 1044, end: 1059 > Window map complete. > kernel BUG at fs/ext2/balloc.c:1158! > invalid opcode: 0000 [#1] PREEMPT SMP KASAN > RIP: 0010:ext2_try_to_allocate_with_rsv.isra.0+0x15c4/0x1800 > Call Trace: > <TASK> > ext2_new_blocks+0x935/0x1690 > ext2_new_block+0x73/0xa0 > ext2_xattr_set2+0x74f/0x1730 > ext2_xattr_set+0x12b6/0x2260 > ext2_xattr_user_set+0x9c/0x110 > __vfs_setxattr+0x139/0x1d0 > __vfs_setxattr_noperm+0xfc/0x370 > __vfs_setxattr_locked+0x205/0x2c0 > vfs_setxattr+0x19d/0x3b0 > do_setxattr+0xff/0x220 > setxattr+0x123/0x150 > path_setxattr+0x193/0x1e0 > __x64_sys_setxattr+0xc8/0x170 > do_syscall_64+0x35/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > Above issue may happens as follows: > setxattr write back > ext2_xattr_set > ext2_xattr_set2 > ext2_new_block > ext2_new_blocks > ext2_try_to_allocate_with_rsv > alloc_new_reservation > --> group=0 [0, 1023] rsv [1016, 1023] > do_writepages > mpage_writepages > write_cache_pages > __mpage_writepage > ext2_get_block > ext2_get_blocks > ext2_alloc_branch > ext2_new_blocks > ext2_try_to_allocate_with_rsv > alloc_new_reservation > -->group=1 [1024, 2047] rsv [1044, 1059] > if ((my_rsv->rsv_start > group_last_block) || > (my_rsv->rsv_end < group_first_block) > rsv_window_dump > BUG(); > Now ext2 mkwrite delay allocate new blocks. So there maybe allocate blocks when > do write back. However, there is no concurrent protection between > ext2_xattr_set() and do_writepages(). > To solve about issue hold '&ei->truncate_mutex' lock when new block for xattr. > > Signed-off-by: Ye Bin <yebin10@huawei.com> Thanks for the patch! I agree the handling of reservation window and its use for block allocation needs to be protected by ei->i_truncate_mutex. However in this particular case of xattr allocation where we want to allocate just one block which is completely independent of file data, I'd rather choose to make ext2_new_blocks() ignore the reservation window (set my_rsv to NULL). There's already a logic at the beginning of ext2_new_blocks() deciding whether to use the reservation window or not and we could extend it - probably by adding flags argument to it a introducing a NORESERVE flag. Also as a preparatory patch, I'd just remove ext2_new_block() and opencode it in the xattr code since it has only that one user anyway. Honza > --- > fs/ext2/balloc.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c > index c8049c90323d..039f655febfd 100644 > --- a/fs/ext2/balloc.c > +++ b/fs/ext2/balloc.c > @@ -1432,8 +1432,14 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal, > ext2_fsblk_t ext2_new_block(struct inode *inode, unsigned long goal, int *errp) > { > unsigned long count = 1; > + struct ext2_inode_info *ei = EXT2_I(inode); > + ext2_fsblk_t ret; > + > + mutex_lock(&ei->truncate_mutex); > + ret = ext2_new_blocks(inode, goal, &count, errp); > + mutex_unlock(&ei->truncate_mutex); > > - return ext2_new_blocks(inode, goal, &count, errp); > + return ret; > } > > #ifdef EXT2FS_DEBUG > -- > 2.31.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ext2: fix race between setxattr and write back 2023-08-14 12:46 ` Jan Kara @ 2023-08-14 15:42 ` yebin (H) 0 siblings, 0 replies; 6+ messages in thread From: yebin (H) @ 2023-08-14 15:42 UTC (permalink / raw) To: Jan Kara; +Cc: linux-ext4, linux-kernel On 2023/8/14 20:46, Jan Kara wrote: > On Fri 11-08-23 11:38:56, Ye Bin wrote: >> There's a issue as follows: >> Block Allocation Reservation Windows Map (ext2_try_to_allocate_with_rsv): >> reservation window 0x000000006f105382 start: 0, end: 0 >> reservation window 0x000000008fd1a555 start: 1044, end: 1059 >> Window map complete. >> kernel BUG at fs/ext2/balloc.c:1158! >> invalid opcode: 0000 [#1] PREEMPT SMP KASAN >> RIP: 0010:ext2_try_to_allocate_with_rsv.isra.0+0x15c4/0x1800 >> Call Trace: >> <TASK> >> ext2_new_blocks+0x935/0x1690 >> ext2_new_block+0x73/0xa0 >> ext2_xattr_set2+0x74f/0x1730 >> ext2_xattr_set+0x12b6/0x2260 >> ext2_xattr_user_set+0x9c/0x110 >> __vfs_setxattr+0x139/0x1d0 >> __vfs_setxattr_noperm+0xfc/0x370 >> __vfs_setxattr_locked+0x205/0x2c0 >> vfs_setxattr+0x19d/0x3b0 >> do_setxattr+0xff/0x220 >> setxattr+0x123/0x150 >> path_setxattr+0x193/0x1e0 >> __x64_sys_setxattr+0xc8/0x170 >> do_syscall_64+0x35/0x80 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> >> Above issue may happens as follows: >> setxattr write back >> ext2_xattr_set >> ext2_xattr_set2 >> ext2_new_block >> ext2_new_blocks >> ext2_try_to_allocate_with_rsv >> alloc_new_reservation >> --> group=0 [0, 1023] rsv [1016, 1023] >> do_writepages >> mpage_writepages >> write_cache_pages >> __mpage_writepage >> ext2_get_block >> ext2_get_blocks >> ext2_alloc_branch >> ext2_new_blocks >> ext2_try_to_allocate_with_rsv >> alloc_new_reservation >> -->group=1 [1024, 2047] rsv [1044, 1059] >> if ((my_rsv->rsv_start > group_last_block) || >> (my_rsv->rsv_end < group_first_block) >> rsv_window_dump >> BUG(); >> Now ext2 mkwrite delay allocate new blocks. So there maybe allocate blocks when >> do write back. However, there is no concurrent protection between >> ext2_xattr_set() and do_writepages(). >> To solve about issue hold '&ei->truncate_mutex' lock when new block for xattr. >> >> Signed-off-by: Ye Bin <yebin10@huawei.com> > Thanks for the patch! I agree the handling of reservation window and its > use for block allocation needs to be protected by ei->i_truncate_mutex. > However in this particular case of xattr allocation where we want to > allocate just one block which is completely independent of file data, I'd > rather choose to make ext2_new_blocks() ignore the reservation window (set > my_rsv to NULL). There's already a logic at the beginning of > ext2_new_blocks() deciding whether to use the reservation window or not and > we could extend it - probably by adding flags argument to it a introducing > a NORESERVE flag. > > Also as a preparatory patch, I'd just remove ext2_new_block() and opencode > it in the xattr code since it has only that one user anyway. > > Honza > Thanks for your suggestion. I will send new version according to your suggestion. > >> --- >> fs/ext2/balloc.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c >> index c8049c90323d..039f655febfd 100644 >> --- a/fs/ext2/balloc.c >> +++ b/fs/ext2/balloc.c >> @@ -1432,8 +1432,14 @@ ext2_fsblk_t ext2_new_blocks(struct inode *inode, ext2_fsblk_t goal, >> ext2_fsblk_t ext2_new_block(struct inode *inode, unsigned long goal, int *errp) >> { >> unsigned long count = 1; >> + struct ext2_inode_info *ei = EXT2_I(inode); >> + ext2_fsblk_t ret; >> + >> + mutex_lock(&ei->truncate_mutex); >> + ret = ext2_new_blocks(inode, goal, &count, errp); >> + mutex_unlock(&ei->truncate_mutex); >> >> - return ext2_new_blocks(inode, goal, &count, errp); >> + return ret; >> } >> >> #ifdef EXT2FS_DEBUG >> -- >> 2.31.1 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] ext2: dump current reservation window info before BUG 2023-08-11 3:38 [PATCH 0/2] fix race between setxattr and write back Ye Bin 2023-08-11 3:38 ` [PATCH 1/2] ext2: " Ye Bin @ 2023-08-11 3:38 ` Ye Bin 2023-08-14 13:08 ` Jan Kara 1 sibling, 1 reply; 6+ messages in thread From: Ye Bin @ 2023-08-11 3:38 UTC (permalink / raw) To: jack, linux-ext4; +Cc: linux-kernel, Ye Bin There's report BUG in 'ext2_try_to_allocate_with_rsv()', although there's now dump of all reservation windows information. But there's unknown which window is being processed.So this is not helpful for locating the issue. To better analyze the problem, dump the information about reservation window that is being processed. Signed-off-by: Ye Bin <yebin10@huawei.com> --- fs/ext2/balloc.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 039f655febfd..0132b8af546e 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -1131,6 +1131,11 @@ ext2_try_to_allocate_with_rsv(struct super_block *sb, unsigned int group, if ((my_rsv->rsv_start > group_last_block) || (my_rsv->rsv_end < group_first_block)) { + ext2_error(sb, __func__, + "Out of group %u range goal %d fsb[%lu,%lu] rsv[%lu, %lu]", + group, grp_goal, group_first_block, + group_last_block, my_rsv->rsv_start, + my_rsv->rsv_end); rsv_window_dump(&EXT2_SB(sb)->s_rsv_window_root, 1); BUG(); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ext2: dump current reservation window info before BUG 2023-08-11 3:38 ` [PATCH 2/2] ext2: dump current reservation window info before BUG Ye Bin @ 2023-08-14 13:08 ` Jan Kara 0 siblings, 0 replies; 6+ messages in thread From: Jan Kara @ 2023-08-14 13:08 UTC (permalink / raw) To: Ye Bin; +Cc: jack, linux-ext4, linux-kernel On Fri 11-08-23 11:38:57, Ye Bin wrote: > There's report BUG in 'ext2_try_to_allocate_with_rsv()', although there's > now dump of all reservation windows information. But there's unknown which > window is being processed.So this is not helpful for locating the issue. > To better analyze the problem, dump the information about reservation window > that is being processed. > > Signed-off-by: Ye Bin <yebin10@huawei.com> > --- > fs/ext2/balloc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c > index 039f655febfd..0132b8af546e 100644 > --- a/fs/ext2/balloc.c > +++ b/fs/ext2/balloc.c > @@ -1131,6 +1131,11 @@ ext2_try_to_allocate_with_rsv(struct super_block *sb, unsigned int group, > > if ((my_rsv->rsv_start > group_last_block) || > (my_rsv->rsv_end < group_first_block)) { > + ext2_error(sb, __func__, > + "Out of group %u range goal %d fsb[%lu,%lu] rsv[%lu, %lu]", ^^^ "Reservation out of group ...." > + group, grp_goal, group_first_block, > + group_last_block, my_rsv->rsv_start, > + my_rsv->rsv_end); > rsv_window_dump(&EXT2_SB(sb)->s_rsv_window_root, 1); > BUG(); > } And we could just bail with error instead of BUG here when we are already changing the code... Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-14 15:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-11 3:38 [PATCH 0/2] fix race between setxattr and write back Ye Bin 2023-08-11 3:38 ` [PATCH 1/2] ext2: " Ye Bin 2023-08-14 12:46 ` Jan Kara 2023-08-14 15:42 ` yebin (H) 2023-08-11 3:38 ` [PATCH 2/2] ext2: dump current reservation window info before BUG Ye Bin 2023-08-14 13:08 ` Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox