From: Jan Kara <jack@suse.cz>
To: Baokun Li <libaokun1@huawei.com>
Cc: linux-ext4@vger.kernel.org, tytso@mit.edu,
adilger.kernel@dilger.ca, jack@suse.cz, ritesh.list@gmail.com,
linux-kernel@vger.kernel.org, yi.zhang@huawei.com,
yangerkun@huawei.com, yukuai3@huawei.com,
Yikebaer Aizezi <yikebaer61@gmail.com>
Subject: Re: [PATCH v2] ext4: fix slab-use-after-free in ext4_es_insert_extent()
Date: Tue, 15 Aug 2023 12:11:31 +0200 [thread overview]
Message-ID: <20230815101131.f5cdeekwmdz5aues@quack3> (raw)
In-Reply-To: <20230815070808.3377171-1-libaokun1@huawei.com>
On Tue 15-08-23 15:08:08, Baokun Li wrote:
> Yikebaer reported an issue:
> ==================================================================
> BUG: KASAN: slab-use-after-free in ext4_es_insert_extent+0xc68/0xcb0
> fs/ext4/extents_status.c:894
> Read of size 4 at addr ffff888112ecc1a4 by task syz-executor/8438
>
> CPU: 1 PID: 8438 Comm: syz-executor Not tainted 6.5.0-rc5 #1
> Call Trace:
> [...]
> kasan_report+0xba/0xf0 mm/kasan/report.c:588
> ext4_es_insert_extent+0xc68/0xcb0 fs/ext4/extents_status.c:894
> ext4_map_blocks+0x92a/0x16f0 fs/ext4/inode.c:680
> ext4_alloc_file_blocks.isra.0+0x2df/0xb70 fs/ext4/extents.c:4462
> ext4_zero_range fs/ext4/extents.c:4622 [inline]
> ext4_fallocate+0x251c/0x3ce0 fs/ext4/extents.c:4721
> [...]
>
> Allocated by task 8438:
> [...]
> kmem_cache_zalloc include/linux/slab.h:693 [inline]
> __es_alloc_extent fs/ext4/extents_status.c:469 [inline]
> ext4_es_insert_extent+0x672/0xcb0 fs/ext4/extents_status.c:873
> ext4_map_blocks+0x92a/0x16f0 fs/ext4/inode.c:680
> ext4_alloc_file_blocks.isra.0+0x2df/0xb70 fs/ext4/extents.c:4462
> ext4_zero_range fs/ext4/extents.c:4622 [inline]
> ext4_fallocate+0x251c/0x3ce0 fs/ext4/extents.c:4721
> [...]
>
> Freed by task 8438:
> [...]
> kmem_cache_free+0xec/0x490 mm/slub.c:3823
> ext4_es_try_to_merge_right fs/ext4/extents_status.c:593 [inline]
> __es_insert_extent+0x9f4/0x1440 fs/ext4/extents_status.c:802
> ext4_es_insert_extent+0x2ca/0xcb0 fs/ext4/extents_status.c:882
> ext4_map_blocks+0x92a/0x16f0 fs/ext4/inode.c:680
> ext4_alloc_file_blocks.isra.0+0x2df/0xb70 fs/ext4/extents.c:4462
> ext4_zero_range fs/ext4/extents.c:4622 [inline]
> ext4_fallocate+0x251c/0x3ce0 fs/ext4/extents.c:4721
> [...]
> ==================================================================
>
> The flow of issue triggering is as follows:
> 1. remove es
> raw es es removed es1
> |-------------------| -> |----|.......|------|
>
> 2. insert es
> es insert es1 merge with es es1 merge with es and free es1
> |----|.......|------| -> |------------|------| -> |-------------------|
>
> es merges with newes, then merges with es1, frees es1, then determines
> if es1->es_len is 0 and triggers a UAF.
>
> The code flow is as follows:
> ext4_es_insert_extent
> es1 = __es_alloc_extent(true);
> es2 = __es_alloc_extent(true);
> __es_remove_extent(inode, lblk, end, NULL, es1)
> __es_insert_extent(inode, &newes, es1) ---> insert es1 to es tree
> __es_insert_extent(inode, &newes, es2)
> ext4_es_try_to_merge_right
> ext4_es_free_extent(inode, es1) ---> es1 is freed
> if (es1 && !es1->es_len)
> // Trigger UAF by determining if es1 is used.
>
> We determine whether es1 or es2 is used immediately after calling
> __es_remove_extent() or __es_insert_extent() to avoid triggering a
> UAF if es1 or es2 is freed.
>
> Reported-by: Yikebaer Aizezi <yikebaer61@gmail.com>
> Closes: https://lore.kernel.org/lkml/CALcu4raD4h9coiyEBL4Bm0zjDwxC2CyPiTwsP3zFuhot6y9Beg@mail.gmail.com
> Fixes: 2a69c450083d ("ext4: using nofail preallocation in ext4_es_insert_extent()")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents_status.c | 44 +++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 9b5b8951afb4..6f7de14c0fa8 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -878,23 +878,29 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
> err1 = __es_remove_extent(inode, lblk, end, NULL, es1);
> if (err1 != 0)
> goto error;
> + /* Free preallocated extent if it didn't get used. */
> + if (es1) {
> + if (!es1->es_len)
> + __es_free_extent(es1);
> + es1 = NULL;
> + }
>
> err2 = __es_insert_extent(inode, &newes, es2);
> if (err2 == -ENOMEM && !ext4_es_must_keep(&newes))
> err2 = 0;
> if (err2 != 0)
> goto error;
> + /* Free preallocated extent if it didn't get used. */
> + if (es2) {
> + if (!es2->es_len)
> + __es_free_extent(es2);
> + es2 = NULL;
> + }
>
> if (sbi->s_cluster_ratio > 1 && test_opt(inode->i_sb, DELALLOC) &&
> (status & EXTENT_STATUS_WRITTEN ||
> status & EXTENT_STATUS_UNWRITTEN))
> __revise_pending(inode, lblk, len);
> -
> - /* es is pre-allocated but not used, free it. */
> - if (es1 && !es1->es_len)
> - __es_free_extent(es1);
> - if (es2 && !es2->es_len)
> - __es_free_extent(es2);
> error:
> write_unlock(&EXT4_I(inode)->i_es_lock);
> if (err1 || err2)
> @@ -1491,8 +1497,12 @@ void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
> */
> write_lock(&EXT4_I(inode)->i_es_lock);
> err = __es_remove_extent(inode, lblk, end, &reserved, es);
> - if (es && !es->es_len)
> - __es_free_extent(es);
> + /* Free preallocated extent if it didn't get used. */
> + if (es) {
> + if (!es->es_len)
> + __es_free_extent(es);
> + es = NULL;
> + }
> write_unlock(&EXT4_I(inode)->i_es_lock);
> if (err)
> goto retry;
> @@ -2047,19 +2057,25 @@ void ext4_es_insert_delayed_block(struct inode *inode, ext4_lblk_t lblk,
> err1 = __es_remove_extent(inode, lblk, lblk, NULL, es1);
> if (err1 != 0)
> goto error;
> + /* Free preallocated extent if it didn't get used. */
> + if (es1) {
> + if (!es1->es_len)
> + __es_free_extent(es1);
> + es1 = NULL;
> + }
>
> err2 = __es_insert_extent(inode, &newes, es2);
> if (err2 != 0)
> goto error;
> + /* Free preallocated extent if it didn't get used. */
> + if (es2) {
> + if (!es2->es_len)
> + __es_free_extent(es2);
> + es2 = NULL;
> + }
>
> if (allocated)
> __insert_pending(inode, lblk);
> -
> - /* es is pre-allocated but not used, free it. */
> - if (es1 && !es1->es_len)
> - __es_free_extent(es1);
> - if (es2 && !es2->es_len)
> - __es_free_extent(es2);
> error:
> write_unlock(&EXT4_I(inode)->i_es_lock);
> if (err1 || err2)
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2023-08-15 10:12 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-15 7:08 [PATCH v2] ext4: fix slab-use-after-free in ext4_es_insert_extent() Baokun Li
2023-08-15 10:11 ` Jan Kara [this message]
2023-08-24 4:53 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230815101131.f5cdeekwmdz5aues@quack3 \
--to=jack@suse.cz \
--cc=adilger.kernel@dilger.ca \
--cc=libaokun1@huawei.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ritesh.list@gmail.com \
--cc=tytso@mit.edu \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yikebaer61@gmail.com \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox