Skip to content

Commit 94dbf7c

Browse files
adam900710kdave
authored andcommitted
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 dd137dd ("btrfs: factor out allocating an array of pages"), but later in commit 544fe4a ("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: 544fe4a ("btrfs: embed a btrfs_bio into struct compressed_bio") CC: [email protected] # 6.4+ Reviewed-by: Filipe Manana <[email protected]> Signed-off-by: Qu Wenruo <[email protected]> Reviewed-by: David Sterba <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 5de0434 commit 94dbf7c

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

fs/btrfs/extent_io.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,8 @@ static void end_bio_extent_readpage(struct btrfs_bio *bbio)
674674
* the array will be skipped
675675
*
676676
* Return: 0 if all pages were able to be allocated;
677-
* -ENOMEM otherwise, and the caller is responsible for freeing all
678-
* non-null page pointers in the array.
677+
* -ENOMEM otherwise, the partially allocated pages would be freed and
678+
* the array slots zeroed
679679
*/
680680
int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array)
681681
{
@@ -694,8 +694,13 @@ int btrfs_alloc_page_array(unsigned int nr_pages, struct page **page_array)
694694
* though alloc_pages_bulk_array() falls back to alloc_page()
695695
* if it could not bulk-allocate. So we must be out of memory.
696696
*/
697-
if (allocated == last)
697+
if (allocated == last) {
698+
for (int i = 0; i < allocated; i++) {
699+
__free_page(page_array[i]);
700+
page_array[i] = NULL;
701+
}
698702
return -ENOMEM;
703+
}
699704

700705
memalloc_retry_wait(GFP_NOFS);
701706
}

0 commit comments

Comments
 (0)