Skip to contentSkip to conversation

Patch "Btrfs: fix memory leak in reading btree blocks" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message Jan 4, 2017

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

Btrfs: fix memory leak in reading btree blocks

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
btrfs-fix-memory-leak-in-reading-btree-blocks.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 2571e739677f1e4c0c63f5ed49adcc0857923625 Mon Sep 17 00:00:00 2001

From: Liu Bo <[email protected]>
Date: Wed, 3 Aug 2016 12:33:01 -0700
Subject: Btrfs: fix memory leak in reading btree blocks

From: Liu Bo <[email protected]>

commit 2571e739677f1e4c0c63f5ed49adcc0857923625 upstream.

So we can read a btree block via readahead or intentional read, and we can end up with a memory leak when something happens as follows,
1) readahead starts to read block A but does not wait for read completion,
2) btree_readpage_end_io_hook finds that block A is corrupted, and it needs to clear all block A's pages' uptodate bit.
3) meanwhile an intentional read kicks in and checks block A's pages' uptodate to decide which page needs to be read.
4) when some pages have the uptodate bit during 3)'s check so
3) doesn't count them for eb->io_pages, but they are later cleared by 2) so we has to readpage on the page, we get the wrong eb->io_pages which results in a memory leak of this block.

This fixes the problem by firstly getting all pages's locking and then checking pages' uptodate bit.

t1(readahead) t2(readahead endio) t3(the following read)
read_extent_buffer_pages end_bio_extent_readpage
for pg in eb: for page 0,1,2 in eb:
if pg is uptodate: btree_readpage_end_io_hook(pg)
num_reads++ if uptodate:
eb->io_pages = num_reads SetPageUptodate(pg) _______________
for pg in eb: for page 3 in eb: read_extent_buffer_pages
if pg is NOT uptodate: btree_readpage_end_io_hook(pg) for pg in eb:
__extent_read_full_page(pg) sanity check reports something wrong if pg is uptodate:
clear_extent_buffer_uptodate(eb) num_reads++
for pg in eb: eb->io_pages = num_reads
ClearPageUptodate(page) _______________
for pg in eb:
if pg is NOT uptodate:
__extent_read_full_page(pg)

So t3's eb->io_pages is not consistent with the number of pages it's reading, and during endio(), atomic_dec_and_test(&eb->io_pages) will get a negative number so that we're not able to free the eb.

---
 fs/btrfs/extent_io.c |    9 +++++++++
 1 file changed, 9 insertions(+)
fs/btrfs/extent_io.c +9 -0
@@ -5294,11 +5294,20 @@ int read_extent_buffer_pages(struct exte
 			lock_page(page);
 		}
 		locked_pages++;
+	}
+	/*
+	 * We need to firstly lock all pages to make sure that
+	 * the uptodate bit of our pages won't be affected by
+	 * clear_extent_buffer_uptodate().
+	 */
+	for (i = start_i; i < num_pages; i++) {
+		page = eb->pages[i];
 		if (!PageUptodate(page)) {
 			num_reads++;
 			all_uptodate = 0;
 		}
 	}
+
 	if (all_uptodate) {
 		if (start_i == 0)
 			set_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags);

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/btrfs-fix-memory-leak-in-reading-btree-blocks.patch queue-4.4/btrfs-bail-out-if-block-group-has-different-mixed-flag.patch queue-4.4/btrfs-return-gracefully-from-balance-if-fs-tree-is-corrupted.patch queue-4.4/btrfs-don-t-bug-during-drop-snapshot.patch queue-4.4/btrfs-fix-memory-leak-in-do_walk_down.patch