Commit 94dbf7c0 authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba

btrfs: free the allocated memory if btrfs_alloc_page_array() fails

[BUG]
If btrfs_alloc_page_array() fail to allocate all pages but part of the
slots, then the partially allocated pages would be leaked in function
btrfs_submit_compressed_read().

[CAUSE]
As explicitly stated, if btrfs_alloc_page_array() returned -ENOMEM,
caller is responsible to free the partially allocated pages.

For the existing call sites, most of them are fine:

- btrfs_raid_bio::stripe_pages
  Handled by free_raid_bio().

- extent_buffer::pages[]
  Handled btrfs_release_extent_buffer_pages().

- scrub_stripe::pages[]
  Handled by release_scrub_stripe().

But there is one exception in btrfs_submit_compressed_read(), if
btrfs_alloc_page_array() failed, we didn't cleanup the array and freed
the array pointer directly.

Initially there is still the error handling in commit dd137dd1
("btrfs: factor out allocating an array of pages"), but later in commit
544fe4a9 ("btrfs: embed a btrfs_bio into struct compressed_bio"),
the error handling is removed, leading to the possible memory leak.

[FIX]
This patch would add back the error handling first, then to prevent such
situation from happening again, also
Make btrfs_alloc_page_array() to free the allocated pages as a extra
safety net, then we don't need to add the error handling to
btrfs_submit_compressed_read().

Fixes: 544fe4a9 ("btrfs: embed a btrfs_bio into struct compressed_bio")
CC: stable@vger.kernel.org # 6.4+
Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 5de0434b
...@@ -674,8 +674,8 @@ static void end_bio_extent_readpage(struct btrfs_bio *bbio) ...@@ -674,8 +674,8 @@ static void end_bio_extent_readpage(struct btrfs_bio *bbio)
* the array will be skipped * the array will be skipped
* *
* Return: 0 if all pages were able to be allocated; * Return: 0 if all pages were able to be allocated;
* -ENOMEM otherwise, and the caller is responsible for freeing all * -ENOMEM otherwise, the partially allocated pages would be freed and
* non-null page pointers in the array. * the array slots zeroed
*/ */
int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array) int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array)
{ {
...@@ -694,8 +694,13 @@ int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array) ...@@ -694,8 +694,13 @@ int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array)
* though alloc_pages_bulk_array() falls back to alloc_page() * though alloc_pages_bulk_array() falls back to alloc_page()
* if it could not bulk-allocate. So we must be out of memory. * if it could not bulk-allocate. So we must be out of memory.
*/ */
if (allocated == last) if (allocated == last) {
for (int i = 0; i < allocated; i++) {
__free_page(page_array[i]);
page_array[i] = NULL;
}
return -ENOMEM; return -ENOMEM;
}
memalloc_retry_wait(GFP_NOFS); memalloc_retry_wait(GFP_NOFS);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment