ext4: don't set up encryption key during jbd2 transaction

commit 4c0d5778385cb3618ff26a561ce41de2b7d9de70 upstream.

Commit a80f7fcf1867 ("ext4: fixup ext4_fc_track_* functions' signature")
extended the scope of the transaction in ext4_unlink() too far, making
it include the call to ext4_find_entry().  However, ext4_find_entry()
can deadlock when called from within a transaction because it may need
to set up the directory's encryption key.

Fix this by restoring the transaction to its original scope.

Reported-by: syzbot+1a748d0007eeac3ab079@syzkaller.appspotmail.com
Fixes: a80f7fcf1867 ("ext4: fixup ext4_fc_track_* functions' signature")
Cc: <stable@vger.kernel.org> # v5.10+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20221106224841.279231-3-ebiggers@kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3afdd99..4e73990 100644 (file)
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3620,8 +3620,8 @@ extern void ext4_initialize_dirent_tail(struct buffer_head *bh,
                                        unsigned int blocksize);
 extern int ext4_handle_dirty_dirblock(handle_t *handle, struct inode *inode,
                                      struct buffer_head *bh);
-extern int __ext4_unlink(handle_t *handle, struct inode *dir, const struct qstr *d_name,
-                        struct inode *inode);
+extern int __ext4_unlink(struct inode *dir, const struct qstr *d_name,
+                        struct inode *inode, struct dentry *dentry);
 extern int __ext4_link(struct inode *dir, struct inode *inode,
                       struct dentry *dentry);
 

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index bec5bc5..1e8be05 100644 (file)
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1402,7 +1402,7 @@ static int ext4_fc_replay_unlink(struct super_block *sb, struct ext4_fc_tl *tl,
                return 0;
        }
 
-       ret = __ext4_unlink(NULL, old_parent, &entry, inode);
+       ret = __ext4_unlink(old_parent, &entry, inode, NULL);
        /* -ENOENT ok coz it might not exist anymore. */
        if (ret == -ENOENT)
                ret = 0;

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index c08c0ab..a789ea9 100644 (file)
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3204,14 +3204,20 @@ end_rmdir:
        return retval;
 }
 
-int __ext4_unlink(handle_t *handle, struct inode *dir, const struct qstr *d_name,
-                 struct inode *inode)
+int __ext4_unlink(struct inode *dir, const struct qstr *d_name,
+                 struct inode *inode,
+                 struct dentry *dentry /* NULL during fast_commit recovery */)
 {
        int retval = -ENOENT;
        struct buffer_head *bh;
        struct ext4_dir_entry_2 *de;
+       handle_t *handle;
        int skip_remove_dentry = 0;
 
+       /*
+        * Keep this outside the transaction; it may have to set up the
+        * directory's encryption key, which isn't GFP_NOFS-safe.
+        */
        bh = ext4_find_entry(dir, d_name, &de, NULL);
        if (IS_ERR(bh))
                return PTR_ERR(bh);
@@ -3228,7 +3234,14 @@ int __ext4_unlink(handle_t *handle, struct inode *dir, const struct qstr *d_name
                if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
                        skip_remove_dentry = 1;
                else
-                       goto out;
+                       goto out_bh;
+       }
+
+       handle = ext4_journal_start(dir, EXT4_HT_DIR,
+                                   EXT4_DATA_TRANS_BLOCKS(dir->i_sb));
+       if (IS_ERR(handle)) {
+               retval = PTR_ERR(handle);
+               goto out_bh;
        }
 
        if (IS_DIRSYNC(dir))
@@ -3237,12 +3250,12 @@ int __ext4_unlink(handle_t *handle, struct inode *dir, const struct qstr *d_name
        if (!skip_remove_dentry) {
                retval = ext4_delete_entry(handle, dir, de, bh);
                if (retval)
-                       goto out;
+                       goto out_handle;
                dir->i_ctime = dir->i_mtime = current_time(dir);
                ext4_update_dx_flag(dir);
                retval = ext4_mark_inode_dirty(handle, dir);
                if (retval)
-                       goto out;
+                       goto out_handle;
        } else {
                retval = 0;
        }
@@ -3255,15 +3268,17 @@ int __ext4_unlink(handle_t *handle, struct inode *dir, const struct qstr *d_name
                ext4_orphan_add(handle, inode);
        inode->i_ctime = current_time(inode);
        retval = ext4_mark_inode_dirty(handle, inode);
-
-out:
+       if (dentry && !retval)
+               ext4_fc_track_unlink(handle, dentry);
+out_handle:
+       ext4_journal_stop(handle);
+out_bh:
        brelse(bh);
        return retval;
 }
 
 static int ext4_unlink(struct inode *dir, struct dentry *dentry)
 {
-       handle_t *handle;
        int retval;
 
        if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
@@ -3281,16 +3296,7 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
        if (retval)
                goto out_trace;
 
-       handle = ext4_journal_start(dir, EXT4_HT_DIR,
-                                   EXT4_DATA_TRANS_BLOCKS(dir->i_sb));
-       if (IS_ERR(handle)) {
-               retval = PTR_ERR(handle);
-               goto out_trace;
-       }
-
-       retval = __ext4_unlink(handle, dir, &dentry->d_name, d_inode(dentry));
-       if (!retval)
-               ext4_fc_track_unlink(handle, dentry);
+       retval = __ext4_unlink(dir, &dentry->d_name, d_inode(dentry), dentry);
 #if IS_ENABLED(CONFIG_UNICODE)
        /* VFS negative dentries are incompatible with Encoding and
         * Case-insensitiveness. Eventually we'll want avoid
@@ -3301,8 +3307,6 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
        if (IS_CASEFOLDED(dir))
                d_invalidate(dentry);
 #endif
-       if (handle)
-               ext4_journal_stop(handle);
 
 out_trace:
        trace_ext4_unlink_exit(dentry, retval);