MIRRORS/linux-imx
1
0
Fork 0
mirror of https://github.com/nxp-imx/linux-imx.git synced 2025-07-07 09:55:19 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

ext4: do not create EA inode under buffer lock

Browse Source 
[ Upstream commit 0a46ef2347 ]

ext4_xattr_set_entry() creates new EA inodes while holding buffer lock
on the external xattr block. This is problematic as it nests all the
allocation locking (which acquires locks on other buffers) under the
buffer lock. This can even deadlock when the filesystem is corrupted and
e.g. quota file is setup to contain xattr block as data block. Move the
allocation of EA inode out of ext4_xattr_set_entry() into the callers.

Reported-by: syzbot+a43d4f48b8397d0e41a9@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/r/20240321162657.27420-2-jack@suse.cz
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Jan Kara 2024-03-21 17:26:50 +01:00 committed by Greg Kroah-Hartman
parent f2a77188a3
commit 737fb7853a
1 changed files with 53 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

113
fs/ext4/xattr.c
View File

@ -1625,6 +1625,7 @@ out_err:
static int ext4_xattr_set_entry(struct ext4_xattr_info *i, static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
struct ext4_xattr_search *s, struct ext4_xattr_search *s,
handle_t *handle, struct inode *inode, handle_t *handle, struct inode *inode,
struct inode *new_ea_inode,
bool is_block) bool is_block)
{ {
struct ext4_xattr_entry *last, *next; struct ext4_xattr_entry *last, *next;
@ -1632,7 +1633,6 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
size_t min_offs = s->end - s->base, name_len = strlen(i->name); size_t min_offs = s->end - s->base, name_len = strlen(i->name);
int in_inode = i->in_inode; int in_inode = i->in_inode;
struct inode *old_ea_inode = NULL; struct inode *old_ea_inode = NULL;
struct inode *new_ea_inode = NULL;
size_t old_size, new_size; size_t old_size, new_size;
int ret; int ret;
@ -1717,38 +1717,11 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
old_ea_inode = NULL; old_ea_inode = NULL;
goto out; goto out;
} }
}
if (i->value && in_inode) {
WARN_ON_ONCE(!i->value_len);
new_ea_inode = ext4_xattr_inode_lookup_create(handle, inode,
i->value, i->value_len);
if (IS_ERR(new_ea_inode)) {
ret = PTR_ERR(new_ea_inode);
new_ea_inode = NULL;
goto out;
}
}
if (old_ea_inode) {
/* We are ready to release ref count on the old_ea_inode. */ /* We are ready to release ref count on the old_ea_inode. */
ret = ext4_xattr_inode_dec_ref(handle, old_ea_inode); ret = ext4_xattr_inode_dec_ref(handle, old_ea_inode);
if (ret) { if (ret)
/* Release newly required ref count on new_ea_inode. */
if (new_ea_inode) {
int err;
err = ext4_xattr_inode_dec_ref(handle,
new_ea_inode);
if (err)
ext4_warning_inode(new_ea_inode,
"dec ref new_ea_inode err=%d",
err);
ext4_xattr_inode_free_quota(inode, new_ea_inode,
i->value_len);
}
goto out; goto out;
}
ext4_xattr_inode_free_quota(inode, old_ea_inode, ext4_xattr_inode_free_quota(inode, old_ea_inode,
le32_to_cpu(here->e_value_size)); le32_to_cpu(here->e_value_size));
@ -1872,7 +1845,6 @@ update_hash:
ret = 0; ret = 0;
out: out:
iput(old_ea_inode); iput(old_ea_inode);
iput(new_ea_inode);
return ret; return ret;
} }
@ -1935,9 +1907,21 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
size_t old_ea_inode_quota = 0; size_t old_ea_inode_quota = 0;
unsigned int ea_ino; unsigned int ea_ino;
#define header(x) ((struct ext4_xattr_header *)(x)) #define header(x) ((struct ext4_xattr_header *)(x))
/* If we need EA inode, prepare it before locking the buffer */
if (i->value && i->in_inode) {
WARN_ON_ONCE(!i->value_len);
ea_inode = ext4_xattr_inode_lookup_create(handle, inode,
i->value, i->value_len);
if (IS_ERR(ea_inode)) {
error = PTR_ERR(ea_inode);
ea_inode = NULL;
goto cleanup;
}
}
if (s->base) { if (s->base) {
int offset = (char *)s->here - bs->bh->b_data; int offset = (char *)s->here - bs->bh->b_data;
@ -1946,6 +1930,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
EXT4_JTR_NONE); EXT4_JTR_NONE);
if (error) if (error)
goto cleanup; goto cleanup;
lock_buffer(bs->bh); lock_buffer(bs->bh);
if (header(s->base)->h_refcount == cpu_to_le32(1)) { if (header(s->base)->h_refcount == cpu_to_le32(1)) {
@ -1972,7 +1957,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
} }
ea_bdebug(bs->bh, "modifying in-place"); ea_bdebug(bs->bh, "modifying in-place");
error = ext4_xattr_set_entry(i, s, handle, inode, error = ext4_xattr_set_entry(i, s, handle, inode,
true /* is_block */); ea_inode, true /* is_block */);
ext4_xattr_block_csum_set(inode, bs->bh); ext4_xattr_block_csum_set(inode, bs->bh);
unlock_buffer(bs->bh); unlock_buffer(bs->bh);
if (error == -EFSCORRUPTED) if (error == -EFSCORRUPTED)
@ -2040,29 +2025,13 @@ clone_block:
s->end = s->base + sb->s_blocksize; s->end = s->base + sb->s_blocksize;
} }
error = ext4_xattr_set_entry(i, s, handle, inode, true /* is_block */); error = ext4_xattr_set_entry(i, s, handle, inode, ea_inode,
true /* is_block */);
if (error == -EFSCORRUPTED) if (error == -EFSCORRUPTED)
goto bad_block; goto bad_block;
if (error) if (error)
goto cleanup; goto cleanup;
if (i->value && s->here->e_value_inum) {
/*
* A ref count on ea_inode has been taken as part of the call to
* ext4_xattr_set_entry() above. We would like to drop this
* extra ref but we have to wait until the xattr block is
* initialized and has its own ref count on the ea_inode.
*/
ea_ino = le32_to_cpu(s->here->e_value_inum);
error = ext4_xattr_inode_iget(inode, ea_ino,
le32_to_cpu(s->here->e_hash),
&ea_inode);
if (error) {
ea_inode = NULL;
goto cleanup;
}
}
inserted: inserted:
if (!IS_LAST_ENTRY(s->first)) { if (!IS_LAST_ENTRY(s->first)) {
new_bh = ext4_xattr_block_cache_find(inode, header(s->base), new_bh = ext4_xattr_block_cache_find(inode, header(s->base),
@ -2215,17 +2184,16 @@ getblk_failed:
cleanup: cleanup:
if (ea_inode) { if (ea_inode) {
int error2; if (error) {
int error2;
error2 = ext4_xattr_inode_dec_ref(handle, ea_inode); error2 = ext4_xattr_inode_dec_ref(handle, ea_inode);
if (error2) if (error2)
ext4_warning_inode(ea_inode, "dec ref error=%d", ext4_warning_inode(ea_inode, "dec ref error=%d",
error2); error2);
/* If there was an error, revert the quota charge. */
if (error)
ext4_xattr_inode_free_quota(inode, ea_inode, ext4_xattr_inode_free_quota(inode, ea_inode,
i_size_read(ea_inode)); i_size_read(ea_inode));
}
iput(ea_inode); iput(ea_inode);
} }
if (ce) if (ce)
@ -2283,14 +2251,38 @@ int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
{ {
struct ext4_xattr_ibody_header *header; struct ext4_xattr_ibody_header *header;
struct ext4_xattr_search *s = &is->s; struct ext4_xattr_search *s = &is->s;
struct inode *ea_inode = NULL;
int error; int error;
if (!EXT4_INODE_HAS_XATTR_SPACE(inode)) if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
return -ENOSPC; return -ENOSPC;
error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */); /* If we need EA inode, prepare it before locking the buffer */
if (error) if (i->value && i->in_inode) {
WARN_ON_ONCE(!i->value_len);
ea_inode = ext4_xattr_inode_lookup_create(handle, inode,
i->value, i->value_len);
if (IS_ERR(ea_inode))
return PTR_ERR(ea_inode);
}
error = ext4_xattr_set_entry(i, s, handle, inode, ea_inode,
false /* is_block */);
if (error) {
if (ea_inode) {
int error2;
error2 = ext4_xattr_inode_dec_ref(handle, ea_inode);
if (error2)
ext4_warning_inode(ea_inode, "dec ref error=%d",
error2);
ext4_xattr_inode_free_quota(inode, ea_inode,
i_size_read(ea_inode));
iput(ea_inode);
}
return error; return error;
}
header = IHDR(inode, ext4_raw_inode(&is->iloc)); header = IHDR(inode, ext4_raw_inode(&is->iloc));
if (!IS_LAST_ENTRY(s->first)) { if (!IS_LAST_ENTRY(s->first)) {
header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC); header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
@ -2299,6 +2291,7 @@ int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
header->h_magic = cpu_to_le32(0); header->h_magic = cpu_to_le32(0);
ext4_clear_inode_state(inode, EXT4_STATE_XATTR); ext4_clear_inode_state(inode, EXT4_STATE_XATTR);
} }
iput(ea_inode);
return 0; return 0;
} }