| 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
| ||
|
Message-ID: <hbb6z62m4km5nm22tyn7xa4bcdbtqqnzwtygbqjosa4gayl2db@gcdpvei7rsjo>
Date: Thu, 6 Nov 2025 10:52:20 +0100
From: Jan Kara <jack@...e.cz>
To: sunyongjian1@...wei.com
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
tytso@....edu, jack@...e.cz, yangerkun@...wei.com, yi.zhang@...wei.com,
libaokun1@...wei.com, chengzhihao1@...wei.com
Subject: Re: [PATCH v2 2/2] ext4: improve integrity checking in
__mb_check_buddy by enhancing order-0 validation
On Thu 06-11-25 14:06:14, Yongjian Sun wrote:
> From: Yongjian Sun <sunyongjian1@...wei.com>
>
> When the MB_CHECK_ASSERT macro is enabled, we found that the
> current validation logic in __mb_check_buddy has a gap in
> detecting certain invalid buddy states, particularly related
> to order-0 (bitmap) bits.
>
> The original logic consists of three steps:
> 1. Validates higher-order buddies: if a higher-order bit is
> set, at most one of the two corresponding lower-order bits
> may be free; if a higher-order bit is clear, both lower-order
> bits must be allocated (and their bitmap bits must be 0).
> 2. For any set bit in order-0, ensures all corresponding
> higher-order bits are not free.
> 3. Verifies that all preallocated blocks (pa) in the group
> have pa_pstart within bounds and their bitmap bits marked as
> allocated.
>
> However, this approach fails to properly validate cases where
> order-0 bits are incorrectly cleared (0), allowing some invalid
> configurations to pass:
>
> corrupt integral
>
> order 3 1 1
> order 2 1 1 1 1
> order 1 1 1 1 1 1 1 1 1
> order 0 0 0 1 1 1 1 1 1 1 1 1 1 1 1 1 1
>
> Here we get two adjacent free blocks at order-0 with inconsistent
> higher-order state, and the right one shows the correct scenario.
>
> The root cause is insufficient validation of order-0 zero bits.
> To fix this and improve completeness without significant performance
> cost, we refine the logic:
>
> 1. Maintain the top-down higher-order validation, but we no longer
> check the cases where the higher-order bit is 0, as this case will
> be covered in step 2.
> 2. Enhance order-0 checking by examining pairs of bits:
> - If either bit in a pair is set (1), all corresponding
> higher-order bits must not be free.
> - If both bits are clear (0), then exactly one of the
> corresponding higher-order bits must be free
> 3. Keep the preallocation (pa) validation unchanged.
>
> This change closes the validation gap, ensuring illegal buddy states
> involving order-0 are correctly detected, while removing redundant
> checks and maintaining efficiency.
>
> Fixes: c9de560ded61f ("ext4: Add multi block allocator for ext4")
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Yongjian Sun <sunyongjian1@...wei.com>
> Reviewed-by: Baokun Li <libaokun1@...wei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/ext4/mballoc.c | 49 +++++++++++++++++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 194a9f995c36..65335248825c 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -682,6 +682,24 @@ do { \
> } \
> } while (0)
>
> +/*
> + * Perform buddy integrity check with the following steps:
> + *
> + * 1. Top-down validation (from highest order down to order 1, excluding order-0 bitmap):
> + * For each pair of adjacent orders, if a higher-order bit is set (indicating a free block),
> + * at most one of the two corresponding lower-order bits may be clear (free).
> + *
> + * 2. Order-0 (bitmap) validation, performed on bit pairs:
> + * - If either bit in a pair is set (1, allocated), then all corresponding higher-order bits
> + * must not be free (0).
> + * - If both bits in a pair are clear (0, free), then exactly one of the corresponding
> + * higher-order bits must be free (0).
> + *
> + * 3. Preallocation (pa) list validation:
> + * For each preallocated block (pa) in the group:
> + * - Verify that pa_pstart falls within the bounds of this block group.
> + * - Ensure the corresponding bit(s) in the order-0 bitmap are marked as allocated (1).
> + */
> static void __mb_check_buddy(struct ext4_buddy *e4b, char *file,
> const char *function, int line)
> {
> @@ -723,15 +741,6 @@ static void __mb_check_buddy(struct ext4_buddy *e4b, char *file,
> continue;
> }
>
> - /* both bits in buddy2 must be 1 */
> - MB_CHECK_ASSERT(mb_test_bit(i << 1, buddy2));
> - MB_CHECK_ASSERT(mb_test_bit((i << 1) + 1, buddy2));
> -
> - for (j = 0; j < (1 << order); j++) {
> - k = (i * (1 << order)) + j;
> - MB_CHECK_ASSERT(
> - !mb_test_bit(k, e4b->bd_bitmap));
> - }
> count++;
> }
> MB_CHECK_ASSERT(e4b->bd_info->bb_counters[order] == count);
> @@ -747,15 +756,21 @@ static void __mb_check_buddy(struct ext4_buddy *e4b, char *file,
> fragments++;
> fstart = i;
> }
> - continue;
> + } else {
> + fstart = -1;
> }
> - fstart = -1;
> - /* check used bits only */
> - for (j = 0; j < e4b->bd_blkbits + 1; j++) {
> - buddy2 = mb_find_buddy(e4b, j, &max2);
> - k = i >> j;
> - MB_CHECK_ASSERT(k < max2);
> - MB_CHECK_ASSERT(mb_test_bit(k, buddy2));
> + if (!(i & 1)) {
> + int in_use, zero_bit_count = 0;
> +
> + in_use = mb_test_bit(i, buddy) || mb_test_bit(i + 1, buddy);
> + for (j = 1; j < e4b->bd_blkbits + 2; j++) {
> + buddy2 = mb_find_buddy(e4b, j, &max2);
> + k = i >> j;
> + MB_CHECK_ASSERT(k < max2);
> + if (!mb_test_bit(k, buddy2))
> + zero_bit_count++;
> + }
> + MB_CHECK_ASSERT(zero_bit_count == !in_use);
> }
> }
> MB_CHECK_ASSERT(!EXT4_MB_GRP_NEED_INIT(e4b->bd_info));
> --
> 2.39.2
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists