f2fs: read COW data with the original inode during atomic write

When updating an atomic-write file, f2fs_write_begin() may read the
previously written data back from the COW inode:
prepare_atomic_write_begin() locates the block in the COW inode and sets
use_cow, and the read bio is then built with the COW inode:

	f2fs_submit_page_read(use_cow ? F2FS_I(inode)->cow_inode : inode,
			      ...);

and f2fs_grab_read_bio() decides whether to schedule fs-layer decryption
(STEP_DECRYPT) for the bio based on that inode via
fscrypt_inode_uses_fs_layer_crypto().

However, the folio being filled belongs to the original inode
(folio->mapping->host == inode), and the data stored in the COW block was
encrypted (or left as plaintext) using the original inode's context, not
the COW inode's -- see f2fs_encrypt_one_page(), which keys off
fio->page->mapping->host.  fscrypt_decrypt_pagecache_blocks() likewise
operates on folio->mapping->host.

The COW inode is created as a tmpfile in the parent directory and inherits
its encryption policy from there.  With test_dummy_encryption the newly
created COW inode gets the dummy policy and becomes encrypted, while a
pre-existing regular file -- created before the policy applied, e.g.
already present in the on-disk image -- stays unencrypted.  The read
path then sets STEP_DECRYPT based on the encrypted COW inode and calls
fscrypt_decrypt_pagecache_blocks() on a folio whose host (the unencrypted
original inode) has a NULL ->i_crypt_info, dereferencing it:

  Oops: general protection fault, probably for non-canonical address ...
  KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
  RIP: 0010:fscrypt_decrypt_pagecache_blocks+0xa0/0x310
  Workqueue: f2fs_post_read_wq f2fs_post_read_work
  Call Trace:
   fscrypt_decrypt_bio+0x1eb/0x340
   f2fs_post_read_work+0xba/0x140
   process_one_work+0x91c/0x1a40
   worker_thread+0x677/0xe90
   kthread+0x2bc/0x3a0

The COW inode is only needed to locate the on-disk block, and that block
address is already resolved into @blkaddr by prepare_atomic_write_begin()
via __find_data_block(cow_inode, ...); f2fs_submit_page_read() then reads
from that physical @blkaddr directly, so the inode argument only selects
the post-read crypto context, not which block is fetched.  Reading with
@inode therefore returns the same (latest, not-yet-committed) COW data,
while making both the fs-layer decryption decision and the inline crypto
path use the correct (original inode's) key.

With the COW inode no longer used at the read site, the use_cow flag has no
remaining consumer; drop it from f2fs_write_begin() and
prepare_atomic_write_begin().

Fixes: 591fc34e1f98 ("f2fs: use cow inode data when updating atomic write")
Cc: stable@vger.kernel.org
Signed-off-by: Mikhail Lobanov <m.lobanov@rosa.ru>
Reviewed-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cea34b4595b1152e73b174056ace020348a16e18..60dea95b0295e4aa67cd268ba5766bac03d2f54c 100644 (file)
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3860,7 +3860,7 @@ unlock_out:
 
 static int prepare_atomic_write_begin(struct f2fs_sb_info *sbi,
                        struct folio *folio, loff_t pos, unsigned int len,
-                       block_t *blk_addr, bool *node_changed, bool *use_cow)
+                       block_t *blk_addr, bool *node_changed)
 {
        struct inode *inode = folio->mapping->host;
        struct inode *cow_inode = F2FS_I(inode)->cow_inode;
@@ -3875,14 +3875,14 @@ static int prepare_atomic_write_begin(struct f2fs_sb_info *sbi,
 
        /* Look for the block in COW inode first */
        err = __find_data_block(cow_inode, index, blk_addr);
-       if (err) {
+       if (err)
                return err;
-       } else if (__is_valid_data_blkaddr(*blk_addr)) {
-               *use_cow = true;
+
+       if (__is_valid_data_blkaddr(*blk_addr))
                return 0;
-       } else if (*blk_addr == NEW_ADDR) {
+
+       if (*blk_addr == NEW_ADDR)
                cow_has_reserved_block = true;
-       }
 
        if (is_inode_flag_set(inode, FI_ATOMIC_REPLACE))
                goto reserve_block;
@@ -3917,7 +3917,6 @@ static int f2fs_write_begin(const struct kiocb *iocb,
        struct folio *folio;
        pgoff_t index = pos >> PAGE_SHIFT;
        bool need_balance = false;
-       bool use_cow = false;
        block_t blkaddr = NULL_ADDR;
        int err = 0;
 
@@ -3980,7 +3979,7 @@ repeat:
 
        if (f2fs_is_atomic_file(inode))
                err = prepare_atomic_write_begin(sbi, folio, pos, len,
-                                       &blkaddr, &need_balance, &use_cow);
+                                       &blkaddr, &need_balance);
        else
                err = prepare_write_begin(sbi, folio, pos, len,
                                        &blkaddr, &need_balance);
@@ -4020,8 +4019,15 @@ repeat:
                        err = -EFSCORRUPTED;
                        goto put_folio;
                }
-               f2fs_submit_page_read(use_cow ? F2FS_I(inode)->cow_inode :
-                                               inode,
+               /*
+                * Although the block may be stored in the COW inode, the folio
+                * belongs to @inode and its data was encrypted (or not) using
+                * @inode's context (see f2fs_encrypt_one_page()).  Read with
+                * @inode so the post-read decryption decision matches the
+                * folio's owner; otherwise an unencrypted @inode whose COW inode
+                * is encrypted hits a NULL ->i_crypt_info on decryption.
+                */
+               f2fs_submit_page_read(inode,
                                      NULL, /* can't write to fsverity files */
                                      folio, blkaddr, 0, true);