linux-ext4 - [PATCH v2] ext4: avoid s_mb_prefetch to be zero in individual scenarios

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1607051143-24508-1-git-send-email-brookxu@tencent.com>
Date:   Fri,  4 Dec 2020 11:05:43 +0800
From:   Chunguang Xu <brookxu.cn@...il.com>
To:     tytso@....edu, adilger.kernel@...ger.ca
Cc:     bzzz@...mcloud.com, linux-ext4@...r.kernel.org
Subject: [PATCH v2] ext4: avoid s_mb_prefetch to be zero in individual scenarios

From: Chunguang Xu <brookxu@...cent.com>

patch cfd7323 introduces block bitmap prefetch, and expects to read
block bitmaps of flex_bg through an IO. However, it seems to ignore
the value range of s_log_groups_per_flex. In the scenario where the
value of s_log_groups_per_flex is greater than 27, s_mb_prefetch or
s_mb_prefetch_limit will overflow, cause a divide zero exception.

In addition, the logic of calculating nr maybe also flawed, because
the size of flexbg is fixed during a single mount, but s_mb_prefetch
can be modified, which causes nr to fail to meet the value condition
of [1, flexbg_size].

 PID: 3873   TASK: ffff88800f11d880  CPU: 2   COMMAND: "executor.4"
 #0 [ffff8880114a6ec0] __show_regs.cold.7 at ffffffff83cf29e2
 #1 [ffff8880114a6f40] do_trap at ffffffff81065c61
 #2 [ffff8880114a6f98] do_error_trap at ffffffff81065d65
 #3 [ffff8880114a6fe0] exc_divide_error at ffffffff83dd2fd4
 #4 [ffff8880114a7000] asm_exc_divide_error at ffffffff83e00872
    [exception RIP: ext4_mb_regular_allocator+3885]
    RIP: ffffffff8191258d  RSP: ffff8880114a70b8  RFLAGS: 00010246
    RAX: 0000000000000000  RBX: 0000000000000000  RCX: ffffffff8191257a
    RDX: 0000000000000000  RSI: 0000000000000000  RDI: 0000000000000005
    RBP: 0000000000000200   R8: ffff88800f11d880   R9: ffffed1001e23b11
    R10: ffff88800f11d887  R11: ffffed1001e23b10  R12: ffff888010147000
    R13: 0000000000000000  R14: 0000000000000002  R15: dffffc0000000000
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #5 [ffff8880114a7260] ext4_mb_new_blocks at ffffffff8191b6ba
 #6 [ffff8880114a7420] ext4_new_meta_blocks at ffffffff81870d6f
 #7 [ffff8880114a74e8] ext4_xattr_block_set at ffffffff819ced37
 #8 [ffff8880114a7758] ext4_xattr_set_handle at ffffffff819d4776
 #9 [ffff8880114a7928] ext4_xattr_set at ffffffff819d501b
    RIP: 000000000045eb29  RSP: 00007ff74e97bc38  RFLAGS: 00000246
    RAX: ffffffffffffffda  RBX: 000000000055bf00  RCX: 000000000045eb29
    RDX: 00000000200000c0  RSI: 0000000020000080  RDI: 0000000020000040
    RBP: 00000000004b068e   R8: 0000000000000001   R9: 0000000000000000
    R10: 0000000000000002  R11: 0000000000000246  R12: 000000000055bf00
    R13: 00007fff50fc111f  R14: 00007ff74e97bdc0  R15: 0000000000022000
    ORIG_RAX: 00000000000000bc  CS: 0033  SS: 002b

To solve this problem, we need to set the upper limit of s_mb_prefetch.
Since we expect to load block bitmaps of a flex_bg through an IO, we
can consider determining a reasonable upper limit among the IO limit
parameters. Comprehensive consideration, we take BLK_MAX_SEGMENT_SIZE.
This maybe a good choice to solve divide zero problem and avoiding
performance degradation.

Reported-by: Tosk Robot <tencent_os_robot@...cent.com>
Signed-off-by: Chunguang Xu <brookxu@...cent.com>
Reviewed-by: Samuel Liao <samuelliao@...cent.com>
Reviewed-by: Andreas Dilger <adilger@...ger.ca>
---
 fs/ext4/mballoc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 24af9ed..e3ea7af 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2395,9 +2395,10 @@ void ext4_mb_prefetch_fini(struct super_block *sb, ext4_group_t group,
 
 				nr = sbi->s_mb_prefetch;
 				if (ext4_has_feature_flex_bg(sb)) {
-					nr = (group / sbi->s_mb_prefetch) *
-						sbi->s_mb_prefetch;
-					nr = nr + sbi->s_mb_prefetch - group;
+					nr = 1 << sbi->s_log_groups_per_flex;
+					if (group & (nr - 1))
+						nr -= group & (nr - 1);
+					nr = min(nr, sbi->s_mb_prefetch);
 				}
 				prefetch_grp = ext4_mb_prefetch(sb, group,
 							nr, &prefetch_ios);
@@ -2732,8 +2733,12 @@ static int ext4_mb_init_backend(struct super_block *sb)
 	}
 
 	if (ext4_has_feature_flex_bg(sb)) {
+		int len;
+
 		/* a single flex group is supposed to be read by a single IO */
-		sbi->s_mb_prefetch = 1 << sbi->s_es->s_log_groups_per_flex;
+		len = min(BLK_MAX_SEGMENT_SIZE >> (sb->s_blocksize_bits - 9),
+			  1 << sbi->s_es->s_log_groups_per_flex);
+		sbi->s_mb_prefetch = len;
 		sbi->s_mb_prefetch *= 8; /* 8 prefetch IOs in flight at most */
 	} else {
 		sbi->s_mb_prefetch = 32;
-- 
1.8.3.1