[PATCH 1/2] f2fs: cache: pin cached block in f2fs_end_cache_writeback()

[PATCH 1/2] f2fs: cache: pin cached block in f2fs_end_cache_writeback()

[Chao Yu]

From: Chao Yu <chao@kernel.org>

Like page cache folios, writeback does not hold an active reference on
the cached block during I/O flight, relying instead on truncation or
shrinker callers to wait on F2FS_BLOCK_WRITEBACK before releasing the entry.

However, in f2fs_end_cache_writeback(), clear_and_wake_up_bit() executes
two distinct steps:
1. clear_bit_unlock(F2FS_BLOCK_WRITEBACK, &entry->state): clears the bit.
2. wake_up_bit(&entry->state, F2FS_BLOCK_WRITEBACK): hashes &entry->state
   to look up the waitqueue and wakes waiting tasks.

Once step 1 clears the bit, a concurrent waiter in f2fs_do_truncate_cache()
or f2fs_do_shrink_cache() is immediately unblocked. The waiter can proceed
to delete the entry from the radix tree, drop the final reference, and
kfree() the entry before step 2 finishes, causing wake_up_bit() to access
freed memory:

CPU 0 (I/O completion)               CPU 1 (Truncation / Shrinker)
- f2fs_cache_write_end_io()
 - f2fs_end_cache_writeback(entry)
  - clear_and_wake_up_bit()
   - clear_bit_unlock(WRITEBACK)
   : bit is cleared!
                                     - f2fs_do_truncate_cache(entry)
                                      - f2fs_cache_wait_writeback(entry)
                                      : sees WRITEBACK cleared!
                                      - radix_tree_delete(&cache->root, ...)
                                      - f2fs_put_cache(entry, true)
                                       - atomic_dec_and_test(&refcount) == 0
                                       - f2fs_do_free_cache(entry)
                                        - kfree(entry->data);
                                        - kfree(entry); <--- FREED!
   - smp_mb__after_atomic()
   - wake_up_bit(&entry->state, ...)
   : Dereferences &entry->state on freed entry! (UAF)

Mirror the logic in folio_end_writeback() by acquiring a temporary reference
via f2fs_cache_get() before clear_and_wake_up_bit() and releasing it with
f2fs_cache_put() once wake_up_bit() completes. This guarantees the entry
can not be freed until wake_up_bit() has finished.

This fixes commit 399410a90ca7 ("f2fs: cache: implement metadata cache").

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/f2fs/cache.c b/fs/f2fs/cache.c
index 38fc5eb17f92..7831e53f5678 100644
--- a/fs/f2fs/cache.c
+++ b/fs/f2fs/cache.c
@@ -117,7 +117,15 @@ void f2fs_end_cache_writeback(struct f2fs_cached_block *entry)
 	 */
 	f2fs_cache_update_tag(entry, F2FS_CACHE_TAG_WRITEBACK,
 						F2FS_CACHE_TAG_NONE);
+	/*
+	 * Writeback does not hold an entry reference of its own, relying
+	 * on truncation to wait for the clearing of F2FS_BLOCK_WRITEBACK.
+	 * But here we must make sure that the entry is not freed and
+	 * reused before clear_and_wake_up_bit().
+	 */
+	f2fs_cache_get(entry);
 	clear_and_wake_up_bit(F2FS_BLOCK_WRITEBACK, &entry->state);
+	f2fs_cache_put(entry);
 }
 
 static int f2fs_cache_refcount(struct f2fs_cached_block *entry)
-- 
2.49.0

[PATCH 2/2] f2fs: cache: pin cached block in f2fs_unlock_cache()

[Chao Yu]

From: Chao Yu <chao@kernel.org>

During asynchronous read readahead (e.g., in f2fs_ra_node_cache()), the
caller drops its own reference via f2fs_put_cache(entry, false) immediately
after issuing the read bio, leaving only the radix tree holding an active
reference during I/O flight.

In f2fs_unlock_cache(), clear_and_wake_up_bit() executes two steps:
1. clear_bit_unlock(F2FS_BLOCK_LOCKED, &entry->state): clears the bit.
2. wake_up_bit(&entry->state, F2FS_BLOCK_LOCKED): hashes &entry->state
   to find the waitqueue and wake sleeping waiters.

Once step 1 clears the bit, a concurrent waiter in f2fs_lock_cache()
(such as in f2fs_truncate_cache() or f2fs_drop_cache()) is immediately
unblocked. The waiter can acquire the lock, delete the entry from the
radix tree, drop the remaining reference, and kfree() the entry before
step 2 completes. This causes wake_up_bit() to dereference freed memory:

CPU 0 (Read I/O completion)         CPU 1 (f2fs_drop_cache / Truncation)
- f2fs_cache_read_end_io()
 - f2fs_unlock_cache(entry)
  - clear_and_wake_up_bit()
   - clear_bit_unlock(LOCKED)
   : bit is cleared!
                                    - f2fs_lock_cache(entry)
                                    : acquires lock!
                                    - f2fs_truncate_locked_cache(entry)
                                     - radix_tree_delete(&cache->root, ...)
                                     - entry->cache = NULL;
                                     - atomic_dec(&entry->refcount);
                                    - f2fs_unlock_cache(entry);
                                    - f2fs_put_cache(entry, false);
                                     - atomic_dec_and_test(&refcount) == 0
                                     - f2fs_do_free_cache(entry)
                                      - kfree(entry->data);
                                      - kfree(entry); <--- FREED!
   - smp_mb__after_atomic()
   - wake_up_bit(&entry->state, ...)
   : Dereferences &entry->state on freed entry! (UAF)

Like commit ("f2fs: cache: pin cached block in f2fs_end_cache_writeback()"),
acquire a temporary reference via f2fs_cache_get() before clear_and_wake_up_bit()
and release it with f2fs_cache_put() once wake_up_bit() completes.

This fixes commit 399410a90ca7 ("f2fs: cache: implement metadata cache")

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/cache.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/f2fs/cache.c b/fs/f2fs/cache.c
index 7831e53f5678..04b5408cbc58 100644
--- a/fs/f2fs/cache.c
+++ b/fs/f2fs/cache.c
@@ -18,6 +18,8 @@
 #include <trace/events/f2fs.h>
 #include "segment.h"
 
+static bool f2fs_cache_put(struct f2fs_cached_block *entry);
+
 void f2fs_cache_wait_writeback_cond(struct f2fs_cached_block *entry,
 					enum page_type type)
 {
@@ -314,7 +316,19 @@ void f2fs_lock_cache(struct f2fs_cached_block *entry)
 
 void f2fs_unlock_cache(struct f2fs_cached_block *entry)
 {
+	/*
+	 * In asynchronous read I/O completion (e.g. from f2fs_ra_node_cache()),
+	 * the I/O completion does not hold a reference of its own. Once
+	 * clear_and_wake_up_bit() clears F2FS_BLOCK_LOCKED, a concurrent waiter
+	 * in f2fs_lock_cache() (e.g. from f2fs_truncate_cache()) can wake up,
+	 * truncate the entry, and drop the final reference before wake_up_bit()
+	 * finishes.
+	 * Pin the entry here to make sure it is not freed before wake_up_bit()
+	 * completes.
+	 */
+	f2fs_cache_get(entry);
 	clear_and_wake_up_bit(F2FS_BLOCK_LOCKED, &entry->state);
+	f2fs_cache_put(entry);
 }
 
 bool f2fs_put_cache(struct f2fs_cached_block *entry, bool unlock)
-- 
2.49.0