* [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search
@ 2022-10-26 4:23 Baokun Li
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Baokun Li @ 2022-10-26 4:23 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, libaokun1
V1->V2:
In patch 2, when imode is not set to S_IFREG, the inode also needs
to be initialized. Otherwise, the check can be bypassed, causing
the BUG_ON. (found in the review by yangerkun)
V2->V3:
a. add EXT4_IGET_BAD flag to prevent unexpected bad inode.
b. check bad quota inode in vfs_setup_quota_inode() instead of in
ext4_quota_enable() for more generic approach to this problem.
c. add helper to check quota inums.
Baokun Li (4):
ext4: fix bug_on in __es_tree_search caused by bad quota inode
ext4: add helper to check quota inums
ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode
ext4: fix bug_on in __es_tree_search caused by bad boot loader inode
fs/ext4/ext4.h | 3 ++-
fs/ext4/inode.c | 8 +++++++-
fs/ext4/ioctl.c | 5 +++--
fs/ext4/super.c | 28 +++++++++++++++++++++++++---
fs/quota/dquot.c | 2 ++
5 files changed, 39 insertions(+), 7 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
@ 2022-10-26 4:23 ` Baokun Li
2022-10-26 9:36 ` Jan Kara
` (2 more replies)
2022-10-26 4:23 ` [PATCH v3 2/4] ext4: add helper to check quota inums Baokun Li
` (3 subsequent siblings)
4 siblings, 3 replies; 15+ messages in thread
From: Baokun Li @ 2022-10-26 4:23 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, libaokun1, linux-fsdevel
We got a issue as fllows:
==================================================================
kernel BUG at fs/ext4/extents_status.c:202!
invalid opcode: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 810 Comm: mount Not tainted 6.1.0-rc1-next-g9631525255e3 #352
RIP: 0010:__es_tree_search.isra.0+0xb8/0xe0
RSP: 0018:ffffc90001227900 EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000077512a0f RCX: 0000000000000000
RDX: 0000000000000002 RSI: 0000000000002a10 RDI: ffff8881004cd0c8
RBP: ffff888177512ac8 R08: 47ffffffffffffff R09: 0000000000000001
R10: 0000000000000001 R11: 00000000000679af R12: 0000000000002a10
R13: ffff888177512d88 R14: 0000000077512a10 R15: 0000000000000000
FS: 00007f4bd76dbc40(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005653bf993cf8 CR3: 000000017bfdf000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
ext4_es_cache_extent+0xe2/0x210
ext4_cache_extents+0xd2/0x110
ext4_find_extent+0x5d5/0x8c0
ext4_ext_map_blocks+0x9c/0x1d30
ext4_map_blocks+0x431/0xa50
ext4_getblk+0x82/0x340
ext4_bread+0x14/0x110
ext4_quota_read+0xf0/0x180
v2_read_header+0x24/0x90
v2_check_quota_file+0x2f/0xa0
dquot_load_quota_sb+0x26c/0x760
dquot_load_quota_inode+0xa5/0x190
ext4_enable_quotas+0x14c/0x300
__ext4_fill_super+0x31cc/0x32c0
ext4_fill_super+0x115/0x2d0
get_tree_bdev+0x1d2/0x360
ext4_get_tree+0x19/0x30
vfs_get_tree+0x26/0xe0
path_mount+0x81d/0xfc0
do_mount+0x8d/0xc0
__x64_sys_mount+0xc0/0x160
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
</TASK>
==================================================================
Above issue may happen as follows:
-------------------------------------
ext4_fill_super
ext4_orphan_cleanup
ext4_enable_quotas
ext4_quota_enable
ext4_iget --> get error inode <5>
ext4_ext_check_inode --> Wrong imode makes it escape inspection
make_bad_inode(inode) --> EXT4_BOOT_LOADER_INO set imode
dquot_load_quota_inode
vfs_setup_quota_inode --> check pass
dquot_load_quota_sb
v2_check_quota_file
v2_read_header
ext4_quota_read
ext4_bread
ext4_getblk
ext4_map_blocks
ext4_ext_map_blocks
ext4_find_extent
ext4_cache_extents
ext4_es_cache_extent
__es_tree_search.isra.0
ext4_es_end --> Wrong extents trigger BUG_ON
In the above issue, s_usr_quota_inum is set to 5, but inode<5> contains
incorrect imode and disordered extents. Because 5 is EXT4_BOOT_LOADER_INO,
the ext4_ext_check_inode check in the ext4_iget function can be bypassed,
finally, the extents that are not checked trigger the BUG_ON in the
__es_tree_search function. To solve this issue, check whether the inode is
bad_inode in vfs_setup_quota_inode().
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/quota/dquot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 0427b44bfee5..f27faf5db554 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2324,6 +2324,8 @@ static int vfs_setup_quota_inode(struct inode *inode, int type)
struct super_block *sb = inode->i_sb;
struct quota_info *dqopt = sb_dqopt(sb);
+ if (is_bad_inode(inode))
+ return -EUCLEAN;
if (!S_ISREG(inode->i_mode))
return -EACCES;
if (IS_RDONLY(inode))
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/4] ext4: add helper to check quota inums
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
@ 2022-10-26 4:23 ` Baokun Li
2022-10-26 9:38 ` Jan Kara
2022-10-26 13:57 ` Jason Yan
2022-10-26 4:23 ` [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode Baokun Li
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Baokun Li @ 2022-10-26 4:23 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, libaokun1
Before quota is enabled, a check on the preset quota inums in
ext4_super_block is added to prevent wrong quota inodes from being loaded.
In addition, when the quota fails to be enabled, the quota type and quota
inum are printed to facilitate fault locating.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/super.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 34b78f380968..0b4060d52d63 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6885,6 +6885,20 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
return err;
}
+static inline bool ext4_check_quota_inum(int type, unsigned long qf_inum)
+{
+ switch (type) {
+ case USRQUOTA:
+ return qf_inum == EXT4_USR_QUOTA_INO;
+ case GRPQUOTA:
+ return qf_inum == EXT4_GRP_QUOTA_INO;
+ case PRJQUOTA:
+ return qf_inum >= EXT4_GOOD_OLD_FIRST_INO;
+ default:
+ BUG();
+ }
+}
+
static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
unsigned int flags)
{
@@ -6901,9 +6915,16 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
if (!qf_inums[type])
return -EPERM;
+ if (!ext4_check_quota_inum(type, qf_inums[type])) {
+ ext4_error(sb, "Bad quota inum: %lu, type: %d",
+ qf_inums[type], type);
+ return -EUCLEAN;
+ }
+
qf_inode = ext4_iget(sb, qf_inums[type], EXT4_IGET_SPECIAL);
if (IS_ERR(qf_inode)) {
- ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
+ ext4_error(sb, "Bad quota inode: %lu, type: %d",
+ qf_inums[type], type);
return PTR_ERR(qf_inode);
}
@@ -6942,8 +6963,9 @@ int ext4_enable_quotas(struct super_block *sb)
if (err) {
ext4_warning(sb,
"Failed to enable quota tracking "
- "(type=%d, err=%d). Please run "
- "e2fsck to fix.", type, err);
+ "(type=%d, err=%d, ino=%lu). "
+ "Please run e2fsck to fix.", type,
+ err, qf_inums[type]);
for (type--; type >= 0; type--) {
struct inode *inode;
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
2022-10-26 4:23 ` [PATCH v3 2/4] ext4: add helper to check quota inums Baokun Li
@ 2022-10-26 4:23 ` Baokun Li
2022-10-26 9:41 ` Jan Kara
2022-10-26 13:59 ` Jason Yan
2022-10-26 4:23 ` [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode Baokun Li
2022-12-06 21:01 ` [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Theodore Ts'o
4 siblings, 2 replies; 15+ messages in thread
From: Baokun Li @ 2022-10-26 4:23 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, libaokun1
There are many places that will get unhappy (and crash) when ext4_iget()
returns a bad inode. However, if iget the boot loader inode, allows a bad
inode to be returned, because the inode may not be initialized. This
mechanism can be used to bypass some checks and cause panic. To solve this
problem, we add a special iget flag EXT4_IGET_BAD. Only with this flag
we'd be returning bad inode from ext4_iget(), otherwise we always return
the error code if the inode is bad inode.(suggested by Jan Kara)
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/ext4.h | 3 ++-
fs/ext4/inode.c | 8 +++++++-
fs/ext4/ioctl.c | 3 ++-
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8d5453852f98..2b574b143bde 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2964,7 +2964,8 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
typedef enum {
EXT4_IGET_NORMAL = 0,
EXT4_IGET_SPECIAL = 0x0001, /* OK to iget a system inode */
- EXT4_IGET_HANDLE = 0x0002 /* Inode # is from a handle */
+ EXT4_IGET_HANDLE = 0x0002, /* Inode # is from a handle */
+ EXT4_IGET_BAD = 0x0004 /* Allow to iget a bad inode */
} ext4_iget_flags;
extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ae3a836dd9d7..f767340229fd 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5043,8 +5043,14 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
if (IS_CASEFOLDED(inode) && !ext4_has_feature_casefold(inode->i_sb))
ext4_error_inode(inode, function, line, 0,
"casefold flag without casefold feature");
- brelse(iloc.bh);
+ if (is_bad_inode(inode) && !(flags & EXT4_IGET_BAD)) {
+ ext4_error_inode(inode, function, line, 0,
+ "bad inode without EXT4_IGET_BAD flag");
+ ret = -EUCLEAN;
+ goto bad_inode;
+ }
+ brelse(iloc.bh);
unlock_new_inode(inode);
return inode;
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index ded535535b27..e0be8026c996 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -375,7 +375,8 @@ static long swap_inode_boot_loader(struct super_block *sb,
blkcnt_t blocks;
unsigned short bytes;
- inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO, EXT4_IGET_SPECIAL);
+ inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO,
+ EXT4_IGET_SPECIAL | EXT4_IGET_BAD);
if (IS_ERR(inode_bl))
return PTR_ERR(inode_bl);
ei_bl = EXT4_I(inode_bl);
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
` (2 preceding siblings ...)
2022-10-26 4:23 ` [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode Baokun Li
@ 2022-10-26 4:23 ` Baokun Li
2022-10-26 10:12 ` Jan Kara
2022-10-26 14:05 ` Jason Yan
2022-12-06 21:01 ` [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Theodore Ts'o
4 siblings, 2 replies; 15+ messages in thread
From: Baokun Li @ 2022-10-26 4:23 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, libaokun1
We got a issue as fllows:
==================================================================
kernel BUG at fs/ext4/extents_status.c:203!
invalid opcode: 0000 [#1] PREEMPT SMP
CPU: 1 PID: 945 Comm: cat Not tainted 6.0.0-next-20221007-dirty #349
RIP: 0010:ext4_es_end.isra.0+0x34/0x42
RSP: 0018:ffffc9000143b768 EFLAGS: 00010203
RAX: 0000000000000000 RBX: ffff8881769cd0b8 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8fc27cf7 RDI: 00000000ffffffff
RBP: ffff8881769cd0bc R08: 0000000000000000 R09: ffffc9000143b5f8
R10: 0000000000000001 R11: 0000000000000001 R12: ffff8881769cd0a0
R13: ffff8881768e5668 R14: 00000000768e52f0 R15: 0000000000000000
FS: 00007f359f7f05c0(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f359f5a2000 CR3: 000000017130c000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__es_tree_search.isra.0+0x6d/0xf5
ext4_es_cache_extent+0xfa/0x230
ext4_cache_extents+0xd2/0x110
ext4_find_extent+0x5d5/0x8c0
ext4_ext_map_blocks+0x9c/0x1d30
ext4_map_blocks+0x431/0xa50
ext4_mpage_readpages+0x48e/0xe40
ext4_readahead+0x47/0x50
read_pages+0x82/0x530
page_cache_ra_unbounded+0x199/0x2a0
do_page_cache_ra+0x47/0x70
page_cache_ra_order+0x242/0x400
ondemand_readahead+0x1e8/0x4b0
page_cache_sync_ra+0xf4/0x110
filemap_get_pages+0x131/0xb20
filemap_read+0xda/0x4b0
generic_file_read_iter+0x13a/0x250
ext4_file_read_iter+0x59/0x1d0
vfs_read+0x28f/0x460
ksys_read+0x73/0x160
__x64_sys_read+0x1e/0x30
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
</TASK>
==================================================================
In the above issue, ioctl invokes the swap_inode_boot_loader function to
swap inode<5> and inode<12>. However, inode<5> contain incorrect imode and
disordered extents, and i_nlink is set to 1. The extents check for inode in
the ext4_iget function can be bypassed bacause 5 is EXT4_BOOT_LOADER_INO.
While links_count is set to 1, the extents are not initialized in
swap_inode_boot_loader. After the ioctl command is executed successfully,
the extents are swapped to inode<12>, in this case, run the `cat` command
to view inode<12>. And Bug_ON is triggered due to the incorrect extents.
When the boot loader inode is not initialized, its imode can be one of the
following:
1) the imode is a bad type, which is marked as bad_inode in ext4_iget and
set to S_IFREG.
2) the imode is good type but not S_IFREG.
3) the imode is S_IFREG.
The BUG_ON may be triggered by bypassing the check in cases 1 and 2.
Therefore, when the boot loader inode is bad_inode or its imode is not
S_IFREG, initialize the inode to avoid triggering the BUG.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index e0be8026c996..8c7a4ae85e99 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -426,7 +426,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
/* Protect extent tree against block allocations via delalloc */
ext4_double_down_write_data_sem(inode, inode_bl);
- if (inode_bl->i_nlink == 0) {
+ if (is_bad_inode(inode_bl) || !S_ISREG(inode_bl->i_mode)) {
/* this inode has never been used as a BOOT_LOADER */
set_nlink(inode_bl, 1);
i_uid_write(inode_bl, 0);
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
@ 2022-10-26 9:36 ` Jan Kara
2022-10-26 13:56 ` Jason Yan
2022-10-26 18:33 ` Chaitanya Kulkarni
2 siblings, 0 replies; 15+ messages in thread
From: Jan Kara @ 2022-10-26 9:36 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yukuai3, linux-fsdevel
On Wed 26-10-22 12:23:07, Baokun Li wrote:
> We got a issue as fllows:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:202!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 810 Comm: mount Not tainted 6.1.0-rc1-next-g9631525255e3 #352
> RIP: 0010:__es_tree_search.isra.0+0xb8/0xe0
> RSP: 0018:ffffc90001227900 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000077512a0f RCX: 0000000000000000
> RDX: 0000000000000002 RSI: 0000000000002a10 RDI: ffff8881004cd0c8
> RBP: ffff888177512ac8 R08: 47ffffffffffffff R09: 0000000000000001
> R10: 0000000000000001 R11: 00000000000679af R12: 0000000000002a10
> R13: ffff888177512d88 R14: 0000000077512a10 R15: 0000000000000000
> FS: 00007f4bd76dbc40(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005653bf993cf8 CR3: 000000017bfdf000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> ext4_es_cache_extent+0xe2/0x210
> ext4_cache_extents+0xd2/0x110
> ext4_find_extent+0x5d5/0x8c0
> ext4_ext_map_blocks+0x9c/0x1d30
> ext4_map_blocks+0x431/0xa50
> ext4_getblk+0x82/0x340
> ext4_bread+0x14/0x110
> ext4_quota_read+0xf0/0x180
> v2_read_header+0x24/0x90
> v2_check_quota_file+0x2f/0xa0
> dquot_load_quota_sb+0x26c/0x760
> dquot_load_quota_inode+0xa5/0x190
> ext4_enable_quotas+0x14c/0x300
> __ext4_fill_super+0x31cc/0x32c0
> ext4_fill_super+0x115/0x2d0
> get_tree_bdev+0x1d2/0x360
> ext4_get_tree+0x19/0x30
> vfs_get_tree+0x26/0xe0
> path_mount+0x81d/0xfc0
> do_mount+0x8d/0xc0
> __x64_sys_mount+0xc0/0x160
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> </TASK>
> ==================================================================
>
> Above issue may happen as follows:
> -------------------------------------
> ext4_fill_super
> ext4_orphan_cleanup
> ext4_enable_quotas
> ext4_quota_enable
> ext4_iget --> get error inode <5>
> ext4_ext_check_inode --> Wrong imode makes it escape inspection
> make_bad_inode(inode) --> EXT4_BOOT_LOADER_INO set imode
> dquot_load_quota_inode
> vfs_setup_quota_inode --> check pass
> dquot_load_quota_sb
> v2_check_quota_file
> v2_read_header
> ext4_quota_read
> ext4_bread
> ext4_getblk
> ext4_map_blocks
> ext4_ext_map_blocks
> ext4_find_extent
> ext4_cache_extents
> ext4_es_cache_extent
> __es_tree_search.isra.0
> ext4_es_end --> Wrong extents trigger BUG_ON
>
> In the above issue, s_usr_quota_inum is set to 5, but inode<5> contains
> incorrect imode and disordered extents. Because 5 is EXT4_BOOT_LOADER_INO,
> the ext4_ext_check_inode check in the ext4_iget function can be bypassed,
> finally, the extents that are not checked trigger the BUG_ON in the
> __es_tree_search function. To solve this issue, check whether the inode is
> bad_inode in vfs_setup_quota_inode().
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Thanks! The patch looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/quota/dquot.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 0427b44bfee5..f27faf5db554 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2324,6 +2324,8 @@ static int vfs_setup_quota_inode(struct inode *inode, int type)
> struct super_block *sb = inode->i_sb;
> struct quota_info *dqopt = sb_dqopt(sb);
>
> + if (is_bad_inode(inode))
> + return -EUCLEAN;
> if (!S_ISREG(inode->i_mode))
> return -EACCES;
> if (IS_RDONLY(inode))
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/4] ext4: add helper to check quota inums
2022-10-26 4:23 ` [PATCH v3 2/4] ext4: add helper to check quota inums Baokun Li
@ 2022-10-26 9:38 ` Jan Kara
2022-10-26 13:57 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jan Kara @ 2022-10-26 9:38 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yukuai3
On Wed 26-10-22 12:23:08, Baokun Li wrote:
> Before quota is enabled, a check on the preset quota inums in
> ext4_super_block is added to prevent wrong quota inodes from being loaded.
> In addition, when the quota fails to be enabled, the quota type and quota
> inum are printed to facilitate fault locating.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Makes sense. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/super.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 34b78f380968..0b4060d52d63 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -6885,6 +6885,20 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
> return err;
> }
>
> +static inline bool ext4_check_quota_inum(int type, unsigned long qf_inum)
> +{
> + switch (type) {
> + case USRQUOTA:
> + return qf_inum == EXT4_USR_QUOTA_INO;
> + case GRPQUOTA:
> + return qf_inum == EXT4_GRP_QUOTA_INO;
> + case PRJQUOTA:
> + return qf_inum >= EXT4_GOOD_OLD_FIRST_INO;
> + default:
> + BUG();
> + }
> +}
> +
> static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
> unsigned int flags)
> {
> @@ -6901,9 +6915,16 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id,
> if (!qf_inums[type])
> return -EPERM;
>
> + if (!ext4_check_quota_inum(type, qf_inums[type])) {
> + ext4_error(sb, "Bad quota inum: %lu, type: %d",
> + qf_inums[type], type);
> + return -EUCLEAN;
> + }
> +
> qf_inode = ext4_iget(sb, qf_inums[type], EXT4_IGET_SPECIAL);
> if (IS_ERR(qf_inode)) {
> - ext4_error(sb, "Bad quota inode # %lu", qf_inums[type]);
> + ext4_error(sb, "Bad quota inode: %lu, type: %d",
> + qf_inums[type], type);
> return PTR_ERR(qf_inode);
> }
>
> @@ -6942,8 +6963,9 @@ int ext4_enable_quotas(struct super_block *sb)
> if (err) {
> ext4_warning(sb,
> "Failed to enable quota tracking "
> - "(type=%d, err=%d). Please run "
> - "e2fsck to fix.", type, err);
> + "(type=%d, err=%d, ino=%lu). "
> + "Please run e2fsck to fix.", type,
> + err, qf_inums[type]);
> for (type--; type >= 0; type--) {
> struct inode *inode;
>
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode
2022-10-26 4:23 ` [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode Baokun Li
@ 2022-10-26 9:41 ` Jan Kara
2022-10-26 13:59 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jan Kara @ 2022-10-26 9:41 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yukuai3
On Wed 26-10-22 12:23:09, Baokun Li wrote:
> There are many places that will get unhappy (and crash) when ext4_iget()
> returns a bad inode. However, if iget the boot loader inode, allows a bad
> inode to be returned, because the inode may not be initialized. This
> mechanism can be used to bypass some checks and cause panic. To solve this
> problem, we add a special iget flag EXT4_IGET_BAD. Only with this flag
> we'd be returning bad inode from ext4_iget(), otherwise we always return
> the error code if the inode is bad inode.(suggested by Jan Kara)
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/ext4.h | 3 ++-
> fs/ext4/inode.c | 8 +++++++-
> fs/ext4/ioctl.c | 3 ++-
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 8d5453852f98..2b574b143bde 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2964,7 +2964,8 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
> typedef enum {
> EXT4_IGET_NORMAL = 0,
> EXT4_IGET_SPECIAL = 0x0001, /* OK to iget a system inode */
> - EXT4_IGET_HANDLE = 0x0002 /* Inode # is from a handle */
> + EXT4_IGET_HANDLE = 0x0002, /* Inode # is from a handle */
> + EXT4_IGET_BAD = 0x0004 /* Allow to iget a bad inode */
> } ext4_iget_flags;
>
> extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ae3a836dd9d7..f767340229fd 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5043,8 +5043,14 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> if (IS_CASEFOLDED(inode) && !ext4_has_feature_casefold(inode->i_sb))
> ext4_error_inode(inode, function, line, 0,
> "casefold flag without casefold feature");
> - brelse(iloc.bh);
> + if (is_bad_inode(inode) && !(flags & EXT4_IGET_BAD)) {
> + ext4_error_inode(inode, function, line, 0,
> + "bad inode without EXT4_IGET_BAD flag");
> + ret = -EUCLEAN;
> + goto bad_inode;
> + }
>
> + brelse(iloc.bh);
> unlock_new_inode(inode);
> return inode;
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index ded535535b27..e0be8026c996 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -375,7 +375,8 @@ static long swap_inode_boot_loader(struct super_block *sb,
> blkcnt_t blocks;
> unsigned short bytes;
>
> - inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO, EXT4_IGET_SPECIAL);
> + inode_bl = ext4_iget(sb, EXT4_BOOT_LOADER_INO,
> + EXT4_IGET_SPECIAL | EXT4_IGET_BAD);
> if (IS_ERR(inode_bl))
> return PTR_ERR(inode_bl);
> ei_bl = EXT4_I(inode_bl);
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode
2022-10-26 4:23 ` [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode Baokun Li
@ 2022-10-26 10:12 ` Jan Kara
2022-10-26 14:05 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jan Kara @ 2022-10-26 10:12 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yukuai3
On Wed 26-10-22 12:23:10, Baokun Li wrote:
> We got a issue as fllows:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:203!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 945 Comm: cat Not tainted 6.0.0-next-20221007-dirty #349
> RIP: 0010:ext4_es_end.isra.0+0x34/0x42
> RSP: 0018:ffffc9000143b768 EFLAGS: 00010203
> RAX: 0000000000000000 RBX: ffff8881769cd0b8 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff8fc27cf7 RDI: 00000000ffffffff
> RBP: ffff8881769cd0bc R08: 0000000000000000 R09: ffffc9000143b5f8
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff8881769cd0a0
> R13: ffff8881768e5668 R14: 00000000768e52f0 R15: 0000000000000000
> FS: 00007f359f7f05c0(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f359f5a2000 CR3: 000000017130c000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> __es_tree_search.isra.0+0x6d/0xf5
> ext4_es_cache_extent+0xfa/0x230
> ext4_cache_extents+0xd2/0x110
> ext4_find_extent+0x5d5/0x8c0
> ext4_ext_map_blocks+0x9c/0x1d30
> ext4_map_blocks+0x431/0xa50
> ext4_mpage_readpages+0x48e/0xe40
> ext4_readahead+0x47/0x50
> read_pages+0x82/0x530
> page_cache_ra_unbounded+0x199/0x2a0
> do_page_cache_ra+0x47/0x70
> page_cache_ra_order+0x242/0x400
> ondemand_readahead+0x1e8/0x4b0
> page_cache_sync_ra+0xf4/0x110
> filemap_get_pages+0x131/0xb20
> filemap_read+0xda/0x4b0
> generic_file_read_iter+0x13a/0x250
> ext4_file_read_iter+0x59/0x1d0
> vfs_read+0x28f/0x460
> ksys_read+0x73/0x160
> __x64_sys_read+0x1e/0x30
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> </TASK>
> ==================================================================
>
> In the above issue, ioctl invokes the swap_inode_boot_loader function to
> swap inode<5> and inode<12>. However, inode<5> contain incorrect imode and
> disordered extents, and i_nlink is set to 1. The extents check for inode in
> the ext4_iget function can be bypassed bacause 5 is EXT4_BOOT_LOADER_INO.
> While links_count is set to 1, the extents are not initialized in
> swap_inode_boot_loader. After the ioctl command is executed successfully,
> the extents are swapped to inode<12>, in this case, run the `cat` command
> to view inode<12>. And Bug_ON is triggered due to the incorrect extents.
>
> When the boot loader inode is not initialized, its imode can be one of the
> following:
> 1) the imode is a bad type, which is marked as bad_inode in ext4_iget and
> set to S_IFREG.
> 2) the imode is good type but not S_IFREG.
> 3) the imode is S_IFREG.
>
> The BUG_ON may be triggered by bypassing the check in cases 1 and 2.
> Therefore, when the boot loader inode is bad_inode or its imode is not
> S_IFREG, initialize the inode to avoid triggering the BUG.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index e0be8026c996..8c7a4ae85e99 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -426,7 +426,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
> /* Protect extent tree against block allocations via delalloc */
> ext4_double_down_write_data_sem(inode, inode_bl);
>
> - if (inode_bl->i_nlink == 0) {
> + if (is_bad_inode(inode_bl) || !S_ISREG(inode_bl->i_mode)) {
> /* this inode has never been used as a BOOT_LOADER */
> set_nlink(inode_bl, 1);
> i_uid_write(inode_bl, 0);
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
2022-10-26 9:36 ` Jan Kara
@ 2022-10-26 13:56 ` Jason Yan
2022-10-26 18:33 ` Chaitanya Kulkarni
2 siblings, 0 replies; 15+ messages in thread
From: Jason Yan @ 2022-10-26 13:56 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, linux-fsdevel
On 2022/10/26 12:23, Baokun Li wrote:
> We got a issue as fllows:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:202!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 810 Comm: mount Not tainted 6.1.0-rc1-next-g9631525255e3 #352
> RIP: 0010:__es_tree_search.isra.0+0xb8/0xe0
> RSP: 0018:ffffc90001227900 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000077512a0f RCX: 0000000000000000
> RDX: 0000000000000002 RSI: 0000000000002a10 RDI: ffff8881004cd0c8
> RBP: ffff888177512ac8 R08: 47ffffffffffffff R09: 0000000000000001
> R10: 0000000000000001 R11: 00000000000679af R12: 0000000000002a10
> R13: ffff888177512d88 R14: 0000000077512a10 R15: 0000000000000000
> FS: 00007f4bd76dbc40(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005653bf993cf8 CR3: 000000017bfdf000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> ext4_es_cache_extent+0xe2/0x210
> ext4_cache_extents+0xd2/0x110
> ext4_find_extent+0x5d5/0x8c0
> ext4_ext_map_blocks+0x9c/0x1d30
> ext4_map_blocks+0x431/0xa50
> ext4_getblk+0x82/0x340
> ext4_bread+0x14/0x110
> ext4_quota_read+0xf0/0x180
> v2_read_header+0x24/0x90
> v2_check_quota_file+0x2f/0xa0
> dquot_load_quota_sb+0x26c/0x760
> dquot_load_quota_inode+0xa5/0x190
> ext4_enable_quotas+0x14c/0x300
> __ext4_fill_super+0x31cc/0x32c0
> ext4_fill_super+0x115/0x2d0
> get_tree_bdev+0x1d2/0x360
> ext4_get_tree+0x19/0x30
> vfs_get_tree+0x26/0xe0
> path_mount+0x81d/0xfc0
> do_mount+0x8d/0xc0
> __x64_sys_mount+0xc0/0x160
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> </TASK>
> ==================================================================
>
> Above issue may happen as follows:
> -------------------------------------
> ext4_fill_super
> ext4_orphan_cleanup
> ext4_enable_quotas
> ext4_quota_enable
> ext4_iget --> get error inode <5>
> ext4_ext_check_inode --> Wrong imode makes it escape inspection
> make_bad_inode(inode) --> EXT4_BOOT_LOADER_INO set imode
> dquot_load_quota_inode
> vfs_setup_quota_inode --> check pass
> dquot_load_quota_sb
> v2_check_quota_file
> v2_read_header
> ext4_quota_read
> ext4_bread
> ext4_getblk
> ext4_map_blocks
> ext4_ext_map_blocks
> ext4_find_extent
> ext4_cache_extents
> ext4_es_cache_extent
> __es_tree_search.isra.0
> ext4_es_end --> Wrong extents trigger BUG_ON
>
> In the above issue, s_usr_quota_inum is set to 5, but inode<5> contains
> incorrect imode and disordered extents. Because 5 is EXT4_BOOT_LOADER_INO,
> the ext4_ext_check_inode check in the ext4_iget function can be bypassed,
> finally, the extents that are not checked trigger the BUG_ON in the
> __es_tree_search function. To solve this issue, check whether the inode is
> bad_inode in vfs_setup_quota_inode().
>
> Signed-off-by: Baokun Li<libaokun1@huawei.com>
> ---
> fs/quota/dquot.c | 2 ++
> 1 file changed, 2 insertions(+)
Looks good,
Reviewed-by: Jason Yan <yanaijie@huawei.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/4] ext4: add helper to check quota inums
2022-10-26 4:23 ` [PATCH v3 2/4] ext4: add helper to check quota inums Baokun Li
2022-10-26 9:38 ` Jan Kara
@ 2022-10-26 13:57 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jason Yan @ 2022-10-26 13:57 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3
On 2022/10/26 12:23, Baokun Li wrote:
> Before quota is enabled, a check on the preset quota inums in
> ext4_super_block is added to prevent wrong quota inodes from being loaded.
> In addition, when the quota fails to be enabled, the quota type and quota
> inum are printed to facilitate fault locating.
>
> Signed-off-by: Baokun Li<libaokun1@huawei.com>
> ---
> fs/ext4/super.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
Looks good,
Reviewed-by: Jason Yan <yanaijie@huawei.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode
2022-10-26 4:23 ` [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode Baokun Li
2022-10-26 9:41 ` Jan Kara
@ 2022-10-26 13:59 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jason Yan @ 2022-10-26 13:59 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3
On 2022/10/26 12:23, Baokun Li wrote:
> There are many places that will get unhappy (and crash) when ext4_iget()
> returns a bad inode. However, if iget the boot loader inode, allows a bad
> inode to be returned, because the inode may not be initialized. This
> mechanism can be used to bypass some checks and cause panic. To solve this
> problem, we add a special iget flag EXT4_IGET_BAD. Only with this flag
> we'd be returning bad inode from ext4_iget(), otherwise we always return
> the error code if the inode is bad inode.(suggested by Jan Kara)
>
> Signed-off-by: Baokun Li<libaokun1@huawei.com>
> ---
> fs/ext4/ext4.h | 3 ++-
> fs/ext4/inode.c | 8 +++++++-
> fs/ext4/ioctl.c | 3 ++-
> 3 files changed, 11 insertions(+), 3 deletions(-)
Looks good,
Reviewed-by: Jason Yan <yanaijie@huawei.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode
2022-10-26 4:23 ` [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode Baokun Li
2022-10-26 10:12 ` Jan Kara
@ 2022-10-26 14:05 ` Jason Yan
1 sibling, 0 replies; 15+ messages in thread
From: Jason Yan @ 2022-10-26 14:05 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3
On 2022/10/26 12:23, Baokun Li wrote:
> We got a issue as fllows:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:203!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 945 Comm: cat Not tainted 6.0.0-next-20221007-dirty #349
> RIP: 0010:ext4_es_end.isra.0+0x34/0x42
> RSP: 0018:ffffc9000143b768 EFLAGS: 00010203
> RAX: 0000000000000000 RBX: ffff8881769cd0b8 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff8fc27cf7 RDI: 00000000ffffffff
> RBP: ffff8881769cd0bc R08: 0000000000000000 R09: ffffc9000143b5f8
> R10: 0000000000000001 R11: 0000000000000001 R12: ffff8881769cd0a0
> R13: ffff8881768e5668 R14: 00000000768e52f0 R15: 0000000000000000
> FS: 00007f359f7f05c0(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f359f5a2000 CR3: 000000017130c000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> __es_tree_search.isra.0+0x6d/0xf5
> ext4_es_cache_extent+0xfa/0x230
> ext4_cache_extents+0xd2/0x110
> ext4_find_extent+0x5d5/0x8c0
> ext4_ext_map_blocks+0x9c/0x1d30
> ext4_map_blocks+0x431/0xa50
> ext4_mpage_readpages+0x48e/0xe40
> ext4_readahead+0x47/0x50
> read_pages+0x82/0x530
> page_cache_ra_unbounded+0x199/0x2a0
> do_page_cache_ra+0x47/0x70
> page_cache_ra_order+0x242/0x400
> ondemand_readahead+0x1e8/0x4b0
> page_cache_sync_ra+0xf4/0x110
> filemap_get_pages+0x131/0xb20
> filemap_read+0xda/0x4b0
> generic_file_read_iter+0x13a/0x250
> ext4_file_read_iter+0x59/0x1d0
> vfs_read+0x28f/0x460
> ksys_read+0x73/0x160
> __x64_sys_read+0x1e/0x30
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> </TASK>
> ==================================================================
>
> In the above issue, ioctl invokes the swap_inode_boot_loader function to
> swap inode<5> and inode<12>. However, inode<5> contain incorrect imode and
> disordered extents, and i_nlink is set to 1. The extents check for inode in
> the ext4_iget function can be bypassed bacause 5 is EXT4_BOOT_LOADER_INO.
> While links_count is set to 1, the extents are not initialized in
> swap_inode_boot_loader. After the ioctl command is executed successfully,
> the extents are swapped to inode<12>, in this case, run the `cat` command
> to view inode<12>. And Bug_ON is triggered due to the incorrect extents.
>
> When the boot loader inode is not initialized, its imode can be one of the
> following:
> 1) the imode is a bad type, which is marked as bad_inode in ext4_iget and
> set to S_IFREG.
> 2) the imode is good type but not S_IFREG.
> 3) the imode is S_IFREG.
>
> The BUG_ON may be triggered by bypassing the check in cases 1 and 2.
> Therefore, when the boot loader inode is bad_inode or its imode is not
> S_IFREG, initialize the inode to avoid triggering the BUG.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks good,
Reviewed-by: Jason Yan <yanaijie@huawei.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
2022-10-26 9:36 ` Jan Kara
2022-10-26 13:56 ` Jason Yan
@ 2022-10-26 18:33 ` Chaitanya Kulkarni
2 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-26 18:33 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yukuai3, linux-fsdevel
On 10/25/2022 9:23 PM, Baokun Li wrote:
> We got a issue as fllows:
> ==================================================================
> kernel BUG at fs/ext4/extents_status.c:202!
> invalid opcode: 0000 [#1] PREEMPT SMP
> CPU: 1 PID: 810 Comm: mount Not tainted 6.1.0-rc1-next-g9631525255e3 #352
> RIP: 0010:__es_tree_search.isra.0+0xb8/0xe0
> RSP: 0018:ffffc90001227900 EFLAGS: 00010202
> RAX: 0000000000000000 RBX: 0000000077512a0f RCX: 0000000000000000
> RDX: 0000000000000002 RSI: 0000000000002a10 RDI: ffff8881004cd0c8
> RBP: ffff888177512ac8 R08: 47ffffffffffffff R09: 0000000000000001
> R10: 0000000000000001 R11: 00000000000679af R12: 0000000000002a10
> R13: ffff888177512d88 R14: 0000000077512a10 R15: 0000000000000000
> FS: 00007f4bd76dbc40(0000)GS:ffff88842fd00000(0000)knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005653bf993cf8 CR3: 000000017bfdf000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> ext4_es_cache_extent+0xe2/0x210
> ext4_cache_extents+0xd2/0x110
> ext4_find_extent+0x5d5/0x8c0
> ext4_ext_map_blocks+0x9c/0x1d30
> ext4_map_blocks+0x431/0xa50
> ext4_getblk+0x82/0x340
> ext4_bread+0x14/0x110
> ext4_quota_read+0xf0/0x180
> v2_read_header+0x24/0x90
> v2_check_quota_file+0x2f/0xa0
> dquot_load_quota_sb+0x26c/0x760
> dquot_load_quota_inode+0xa5/0x190
> ext4_enable_quotas+0x14c/0x300
> __ext4_fill_super+0x31cc/0x32c0
> ext4_fill_super+0x115/0x2d0
> get_tree_bdev+0x1d2/0x360
> ext4_get_tree+0x19/0x30
> vfs_get_tree+0x26/0xe0
> path_mount+0x81d/0xfc0
> do_mount+0x8d/0xc0
> __x64_sys_mount+0xc0/0x160
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> </TASK>
> ==================================================================
>
> Above issue may happen as follows:
> -------------------------------------
> ext4_fill_super
> ext4_orphan_cleanup
> ext4_enable_quotas
> ext4_quota_enable
> ext4_iget --> get error inode <5>
> ext4_ext_check_inode --> Wrong imode makes it escape inspection
> make_bad_inode(inode) --> EXT4_BOOT_LOADER_INO set imode
> dquot_load_quota_inode
> vfs_setup_quota_inode --> check pass
> dquot_load_quota_sb
> v2_check_quota_file
> v2_read_header
> ext4_quota_read
> ext4_bread
> ext4_getblk
> ext4_map_blocks
> ext4_ext_map_blocks
> ext4_find_extent
> ext4_cache_extents
> ext4_es_cache_extent
> __es_tree_search.isra.0
> ext4_es_end --> Wrong extents trigger BUG_ON
>
> In the above issue, s_usr_quota_inum is set to 5, but inode<5> contains
> incorrect imode and disordered extents. Because 5 is EXT4_BOOT_LOADER_INO,
> the ext4_ext_check_inode check in the ext4_iget function can be bypassed,
> finally, the extents that are not checked trigger the BUG_ON in the
> __es_tree_search function. To solve this issue, check whether the inode is
> bad_inode in vfs_setup_quota_inode().
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
The analysis and the provided call chain looks really good, only thing
non-blocker is missing that maintainer might want it version history.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
` (3 preceding siblings ...)
2022-10-26 4:23 ` [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode Baokun Li
@ 2022-12-06 21:01 ` Theodore Ts'o
4 siblings, 0 replies; 15+ messages in thread
From: Theodore Ts'o @ 2022-12-06 21:01 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: Theodore Ts'o, linux-kernel, yi.zhang, ritesh.list,
adilger.kernel, yukuai3, jack
On Wed, 26 Oct 2022 12:23:06 +0800, Baokun Li wrote:
> V1->V2:
> In patch 2, when imode is not set to S_IFREG, the inode also needs
> to be initialized. Otherwise, the check can be bypassed, causing
> the BUG_ON. (found in the review by yangerkun)
> V2->V3:
> a. add EXT4_IGET_BAD flag to prevent unexpected bad inode.
> b. check bad quota inode in vfs_setup_quota_inode() instead of in
> ext4_quota_enable() for more generic approach to this problem.
> c. add helper to check quota inums.
>
> [...]
Applied, thanks!
[1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode
commit: c7e9666f28ba9bdeeb99fb0c60a27dbb88f452f4
[2/4] ext4: add helper to check quota inums
commit: 9c4883f1b41181f2096e2ee4e98111008b77165c
[3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode
commit: 10f525fda2faff81f6cfce2a6bc4b50a5254d9ea
[4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode
commit: db14233edaf579153d8c92bf3a0ba27ceb87eabc
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-12-06 21:02 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26 4:23 [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Baokun Li
2022-10-26 4:23 ` [PATCH v3 1/4] ext4: fix bug_on in __es_tree_search caused by bad quota inode Baokun Li
2022-10-26 9:36 ` Jan Kara
2022-10-26 13:56 ` Jason Yan
2022-10-26 18:33 ` Chaitanya Kulkarni
2022-10-26 4:23 ` [PATCH v3 2/4] ext4: add helper to check quota inums Baokun Li
2022-10-26 9:38 ` Jan Kara
2022-10-26 13:57 ` Jason Yan
2022-10-26 4:23 ` [PATCH v3 3/4] ext4: add EXT4_IGET_BAD flag to prevent unexpected bad inode Baokun Li
2022-10-26 9:41 ` Jan Kara
2022-10-26 13:59 ` Jason Yan
2022-10-26 4:23 ` [PATCH v3 4/4] ext4: fix bug_on in __es_tree_search caused by bad boot loader inode Baokun Li
2022-10-26 10:12 ` Jan Kara
2022-10-26 14:05 ` Jason Yan
2022-12-06 21:01 ` [PATCH v3 0/4] ext4: fix two bug_on in __es_tree_search Theodore Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.