diff mbox series

[Concept,10/26] ext4l: Fix a few problems with handling bh_cache

Message ID 20251231223008.3251711-11-sjg@u-boot.org
State New
Headers
Series ext4l: Add write support (part L) |

Commit Message

Simon Glass Dec. 31, 2025, 10:29 p.m. UTC 
  From: Simon Glass <simon.glass@canonical.com>

Several buffer cache issues cause problems when remounting:

1. bh_cache_insert() only checks block number, but the same block can
   be read with different sizes (e.g. superblock at 1K vs 4K). Check
   both block number and size when determining if already cached.

2. bh_cache_clear() leaves stale buffer references, causing memory
   leaks. Force the reference count to 1 before releasing since ext4
   code won't access these buffers after unmount.

3. brelse() calls free_buffer_head() when the reference count reaches
   zero. However, buffer heads remain in the cache and are freed by
   bh_cache_clear() during unmount. This causes a double-free. Fix this
   by having brelse() only decrement the count, without freeing.

Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 fs/ext4l/support.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/fs/ext4l/support.c b/fs/ext4l/support.c
index 7f2bea4ca06..3133a4ebead 100644
--- a/fs/ext4l/support.c
+++ b/fs/ext4l/support.c
@@ -258,9 +258,10 @@  static void bh_cache_insert(struct buffer_head *bh)
 	unsigned int hash = bh_cache_hash(bh->b_blocknr);
 	struct bh_cache_entry *entry;
 
-	/* Check if already in cache */
+	/* Check if already in cache - must match block AND size */
 	for (entry = bh_cache[hash]; entry; entry = entry->next) {
-		if (entry->bh && entry->bh->b_blocknr == bh->b_blocknr)
+		if (entry->bh && entry->bh->b_blocknr == bh->b_blocknr &&
+		    entry->bh->b_size == bh->b_size)
 			return;  /* Already cached */
 	}
 
@@ -316,7 +317,12 @@  void bh_cache_clear(void)
 				struct buffer_head *bh = entry->bh;
 
 				bh_clear_stale_jbd(bh);
-				/* Release the cache's reference */
+				/*
+				 * Force count to 1 so the buffer will be freed.
+				 * On unmount, ext4 code won't access these
+				 * buffers again, so extra references are stale.
+				 */
+				atomic_set(&bh->b_count, 1);
 				if (atomic_dec_and_test(&bh->b_count))
 					free_buffer_head(bh);
 			}
@@ -629,14 +635,24 @@  struct buffer_head *sb_bread(struct super_block *sb, sector_t block)
 /**
  * brelse() - Release a buffer_head
  * @bh: Buffer head to release
+ *
+ * Decrements the reference count on the buffer. Buffer heads are cached
+ * and only freed by bh_cache_clear() on unmount, so this just decrements
+ * the count without freeing.
  */
 void brelse(struct buffer_head *bh)
 {
 	if (!bh)
 		return;
 
-	if (atomic_dec_and_test(&bh->b_count))
-		free_buffer_head(bh);
+	/*
+	 * If buffer has JBD attached, don't let ref count go to zero.
+	 * The journal owns a reference and will clean up properly.
+	 */
+	if (buffer_jbd(bh) && atomic_read(&bh->b_count) <= 1)
+		return;
+
+	atomic_dec(&bh->b_count);
 }
 
 /**