Commit 4c0d5778 authored by Eric Biggers's avatar Eric Biggers Committed by Theodore Ts'o
Browse files

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



Commit a80f7fcf ("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: default avatar <syzbot+1a748d0007eeac3ab079@syzkaller.appspotmail.com>
Fixes: a80f7fcf ("ext4: fixup ext4_fc_track_* functions' signature")
Cc: <stable@vger.kernel.org> # v5.10+
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Link: https://lore.kernel.org/r/20221106224841.279231-3-ebiggers@kernel.org


Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 0fbcb525

fs/ext4/ext4.h

+2 −2
Original line number Diff line number Diff line
@@ -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);

fs/ext4/fast_commit.c

+1 −1
Original line number Diff line number Diff line
@@ -1397,7 +1397,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;

fs/ext4/namei.c

+24 −20
Original line number Diff line number Diff line
@@ -3204,14 +3204,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);
@@ -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);