2023-01-07 20:52:31

by Eric Biggers

[permalink] [raw]
Subject: [PATCH 5.10 0/2] Selected ext4 fast-commit fixes for 5.10-stable

The recent ext4 fast-commit fixes with 'Cc stable' didn't apply to 5.10
due to conflicts. Since the fast-commit support in 5.10 is rudimentary
and hard to backport fixes too, this series backports the two most
important fixes only. Please apply to 5.10-stable.

Eric Biggers (2):
ext4: disable fast-commit of encrypted dir operations
ext4: don't set up encryption key during jbd2 transaction

fs/ext4/ext4.h | 4 ++--
fs/ext4/fast_commit.c | 42 +++++++++++++++++++++--------------
fs/ext4/fast_commit.h | 1 +
fs/ext4/namei.c | 44 ++++++++++++++++++++-----------------
include/trace/events/ext4.h | 7 ++++--
5 files changed, 57 insertions(+), 41 deletions(-)

--
2.39.0


2023-01-07 20:57:17

by Eric Biggers

[permalink] [raw]
Subject: [PATCH 5.10 2/2] ext4: don't set up encryption key during jbd2 transaction

From: Eric Biggers <[email protected]>

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: [email protected]
Fixes: a80f7fcf1867 ("ext4: fixup ext4_fc_track_* functions' signature")
Cc: <[email protected]> # v5.10+
Signed-off-by: Eric Biggers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Theodore Ts'o <[email protected]>
---
fs/ext4/ext4.h | 4 ++--
fs/ext4/fast_commit.c | 2 +-
fs/ext4/namei.c | 44 +++++++++++++++++++++++--------------------
3 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index fb9c9e1813bc5..81dc61f1c557f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3486,8 +3486,8 @@ extern int ext4_handle_dirty_dirblock(handle_t *handle, struct inode *inode,
extern int ext4_ci_compare(const struct inode *parent,
const struct qstr *fname,
const struct qstr *entry, bool quick);
-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 e26020598e194..be96f5ccc55dd 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1295,7 +1295,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 c17d5f399f9ea..e296b3587bb38 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3244,14 +3244,20 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
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);
@@ -3268,7 +3274,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))
@@ -3277,12 +3290,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;
}
@@ -3295,15 +3308,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))))
@@ -3321,16 +3336,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);
#ifdef CONFIG_UNICODE
/* VFS negative dentries are incompatible with Encoding and
* Case-insensitiveness. Eventually we'll want avoid
@@ -3341,8 +3347,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);
--
2.39.0

2023-01-12 12:49:39

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5.10 0/2] Selected ext4 fast-commit fixes for 5.10-stable

On Sat, Jan 07, 2023 at 12:37:11PM -0800, Eric Biggers wrote:
> The recent ext4 fast-commit fixes with 'Cc stable' didn't apply to 5.10
> due to conflicts. Since the fast-commit support in 5.10 is rudimentary
> and hard to backport fixes too, this series backports the two most
> important fixes only. Please apply to 5.10-stable.
>
> Eric Biggers (2):
> ext4: disable fast-commit of encrypted dir operations
> ext4: don't set up encryption key during jbd2 transaction
>
> fs/ext4/ext4.h | 4 ++--
> fs/ext4/fast_commit.c | 42 +++++++++++++++++++++--------------
> fs/ext4/fast_commit.h | 1 +
> fs/ext4/namei.c | 44 ++++++++++++++++++++-----------------
> include/trace/events/ext4.h | 7 ++++--
> 5 files changed, 57 insertions(+), 41 deletions(-)
>
> --
> 2.39.0
>

All now queued up, thanks.

greg k-h