linux-kernel - Re: [PATCH v2] btrfs: Fix slab-use-after-free Read in add_ra_bio

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-prev] [day] [month] [year] [list]

Message-ID: <CAL3q7H6NisGBRqccRv3CiTDMJS0M7ZnvSrEDDHSOUn2TaWBBDw@mail.gmail.com>
Date: Thu, 11 Jul 2024 07:38:40 +0100
From: Filipe Manana <fdmanana@...nel.org>
To: Pei Li <peili.dev@...il.com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>, 
	Qu Wenruo <wqu@...e.com>, linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org, skhan@...uxfoundation.org, 
	syzkaller-bugs@...glegroups.com, 
	linux-kernel-mentees@...ts.linuxfoundation.org, 
	syzbot+853d80cba98ce1157ae6@...kaller.appspotmail.com
Subject: Re: [PATCH v2] btrfs: Fix slab-use-after-free Read in add_ra_bio_pages

On Thu, Jul 11, 2024 at 5:29 AM Pei Li <peili.dev@...il.com> wrote:
>
> We are accessing the start and len field in em after it is free'd.
>
> This patch moves the line accessing the free'd values in em before
> they were free'd so we won't access free'd memory.
>
> Reported-by: syzbot+853d80cba98ce1157ae6@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=853d80cba98ce1157ae6
> Signed-off-by: Pei Li <peili.dev@...il.com>
> ---
> Syzbot reported the following error:
> BUG: KASAN: slab-use-after-free in add_ra_bio_pages.constprop.0.isra.0+0xf03/0xfb0 fs/btrfs/compression.c:529
>
> This is because we are reading the values from em right after freeing it
> before through free_extent_map(em).
>
> This patch moves the line accessing the free'd values in em before
> they were free'd so we won't access free'd memory.
>
> Fixes: 6a4049102055 ("btrfs: subpage: make add_ra_bio_pages() compatible")

This type of useful information should be in the changelog, not after the ---

And btw, this was already fixed last week and it's in for-next:

https://github.com/btrfs/linux/commit/aaa2c8b3f54e7b4f31616fd03bb302cc17cccf39
https://lore.kernel.org/linux-btrfs/20240704171031.GX21023@twin.jikos.cz/T/#m9a92a5d980230323ec351a24adf9a3738cfbbc40

Thanks.

> ---
> Changes in v2:
> - Adapt Qu's suggestion to move the read-after-free line before freeing
> - Cc stable kernel
> - Link to v1: https://lore.kernel.org/r/20240710-bug11-v1-1-aa02297fbbc9@gmail.com
> ---
>  fs/btrfs/compression.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 6441e47d8a5e..f271df10ef1c 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -514,6 +514,8 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>                         put_page(page);
>                         break;
>                 }
> +               add_size = min(em->start + em->len, page_end + 1) - cur;
> +
>                 free_extent_map(em);
>
>                 if (page->index == end_index) {
> @@ -526,7 +528,6 @@ static noinline int add_ra_bio_pages(struct inode *inode,
>                         }
>                 }
>
> -               add_size = min(em->start + em->len, page_end + 1) - cur;
>                 ret = bio_add_page(orig_bio, page, add_size, offset_in_page(cur));
>                 if (ret != add_size) {
>                         unlock_extent(tree, cur, page_end, NULL);
>
> ---
> base-commit: 563a50672d8a86ec4b114a4a2f44d6e7ff855f5b
> change-id: 20240710-bug11-a8ac18afb724
>
> Best regards,
> --
> Pei Li <peili.dev@...il.com>
>
>