From: Ye Bin <[email protected]>
This patchset fix two issues:
1. Patch [1]-[4] fix WARNING in ext4_expand_extra_isize_ea.
2. Patch [6] fix inode leak in 'ext4_xattr_inode_create()'.
3. Patch [5] is cleanup.
Ye Bin (6):
ext4: fix WARNING in ext4_expand_extra_isize_ea
ext4: add primary check extended attribute inode in
ext4_xattr_check_entries()
ext4: remove unnessary size check in ext4_xattr_inode_get()
ext4: allocate extended attribute value in vmalloc area
ext4: rename xattr_find_entry() and __xattr_check_inode()
ext4: fix inode leak in 'ext4_xattr_inode_create()'
fs/ext4/xattr.c | 82 ++++++++++++++++++++++++++++++++-----------------
fs/ext4/xattr.h | 11 ++-----
2 files changed, 57 insertions(+), 36 deletions(-)
--
2.31.1
From: Ye Bin <[email protected]>
There is issue as follows when do setxattr with inject fault:
[localhost]#fsck.ext4 -fn /dev/sda
e2fsck 1.46.6-rc1 (12-Sep-2022)
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Unattached zero-length inode 15. Clear? no
Unattached inode 15
Connect to /lost+found? no
Pass 5: Checking group summary information
/dev/sda: ********** WARNING: Filesystem still has errors **********
/dev/sda: 15/655360 files (0.0% non-contiguous), 66755/2621440 blocks
Above issue occur in 'ext4_xattr_inode_create()', if 'ext4_mark_inode_dirty()'
failed need to drop inode's i_nlink. Or will lead to inode leak.
Signed-off-by: Ye Bin <[email protected]>
---
fs/ext4/xattr.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 003fe1f2d6a8..734f787ae7ed 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1464,6 +1464,9 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
if (!err)
err = ext4_inode_attach_jinode(ea_inode);
if (err) {
+ if (ext4_xattr_inode_dec_ref(handle, ea_inode))
+ ext4_warning_inode(ea_inode,
+ "cleanup dec ref error %d", err);
iput(ea_inode);
return ERR_PTR(err);
}
--
2.31.1
From: Ye Bin <[email protected]>
xattr_find_entry() and __xattr_check_inode() is in EXT4 xattr module. so
add 'ext4' prefix to unify name style.
Signed-off-by: Ye Bin <[email protected]>
---
fs/ext4/xattr.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index efa623658c12..003fe1f2d6a8 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -290,7 +290,7 @@ __ext4_xattr_check_block(struct inode *inode, struct buffer_head *bh,
static int
-__xattr_check_inode(struct inode *inode, struct ext4_xattr_ibody_header *header,
+__ext4_xattr_check_inode(struct inode *inode, struct ext4_xattr_ibody_header *header,
void *end, const char *function, unsigned int line)
{
int error = -EFSCORRUPTED;
@@ -307,11 +307,11 @@ __xattr_check_inode(struct inode *inode, struct ext4_xattr_ibody_header *header,
return error;
}
-#define xattr_check_inode(inode, header, end) \
- __xattr_check_inode((inode), (header), (end), __func__, __LINE__)
+#define ext4_xattr_check_inode(inode, header, end) \
+ __ext4_xattr_check_inode((inode), (header), (end), __func__, __LINE__)
static int
-xattr_find_entry(struct inode *inode, struct ext4_xattr_entry **pentry,
+ext4_xattr_find_entry(struct inode *inode, struct ext4_xattr_entry **pentry,
void *end, int name_index, const char *name, int sorted)
{
struct ext4_xattr_entry *entry, *next;
@@ -577,7 +577,7 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
ext4_xattr_block_cache_insert(ea_block_cache, bh);
entry = BFIRST(bh);
end = bh->b_data + bh->b_size;
- error = xattr_find_entry(inode, &entry, end, name_index, name, 1);
+ error = ext4_xattr_find_entry(inode, &entry, end, name_index, name, 1);
if (error)
goto cleanup;
size = le32_to_cpu(entry->e_value_size);
@@ -628,11 +628,11 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
raw_inode = ext4_raw_inode(&iloc);
header = IHDR(inode, raw_inode);
end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
- error = xattr_check_inode(inode, header, end);
+ error = ext4_xattr_check_inode(inode, header, end);
if (error)
goto cleanup;
entry = IFIRST(header);
- error = xattr_find_entry(inode, &entry, end, name_index, name, 0);
+ error = ext4_xattr_find_entry(inode, &entry, end, name_index, name, 0);
if (error)
goto cleanup;
size = le32_to_cpu(entry->e_value_size);
@@ -773,7 +773,7 @@ ext4_xattr_ibody_list(struct dentry *dentry, char *buffer, size_t buffer_size)
raw_inode = ext4_raw_inode(&iloc);
header = IHDR(inode, raw_inode);
end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
- error = xattr_check_inode(inode, header, end);
+ error = ext4_xattr_check_inode(inode, header, end);
if (error)
goto cleanup;
error = ext4_xattr_list_entries(dentry, IFIRST(header),
@@ -859,7 +859,7 @@ int ext4_get_inode_usage(struct inode *inode, qsize_t *usage)
raw_inode = ext4_raw_inode(&iloc);
header = IHDR(inode, raw_inode);
end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
- ret = xattr_check_inode(inode, header, end);
+ ret = ext4_xattr_check_inode(inode, header, end);
if (ret)
goto out;
@@ -1862,7 +1862,7 @@ ext4_xattr_block_find(struct inode *inode, struct ext4_xattr_info *i,
bs->s.first = BFIRST(bs->bh);
bs->s.end = bs->bh->b_data + bs->bh->b_size;
bs->s.here = bs->s.first;
- error = xattr_find_entry(inode, &bs->s.here, bs->s.end,
+ error = ext4_xattr_find_entry(inode, &bs->s.here, bs->s.end,
i->name_index, i->name, 1);
if (error && error != -ENODATA)
return error;
@@ -2222,11 +2222,11 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
is->s.here = is->s.first;
is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
- error = xattr_check_inode(inode, header, is->s.end);
+ error = ext4_xattr_check_inode(inode, header, is->s.end);
if (error)
return error;
/* Find the named attribute. */
- error = xattr_find_entry(inode, &is->s.here, is->s.end,
+ error = ext4_xattr_find_entry(inode, &is->s.here, is->s.end,
i->name_index, i->name, 0);
if (error && error != -ENODATA)
return error;
@@ -2742,7 +2742,7 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
min_offs = end - base;
total_ino = sizeof(struct ext4_xattr_ibody_header) + sizeof(u32);
- error = xattr_check_inode(inode, header, end);
+ error = ext4_xattr_check_inode(inode, header, end);
if (error)
goto cleanup;
--
2.31.1
On Tue 06-12-22 09:58:05, Ye Bin wrote:
> From: Ye Bin <[email protected]>
>
> xattr_find_entry() and __xattr_check_inode() is in EXT4 xattr module. so
> add 'ext4' prefix to unify name style.
>
> Signed-off-by: Ye Bin <[email protected]>
Looks nice. Just one nit below:
> @@ -1862,7 +1862,7 @@ ext4_xattr_block_find(struct inode *inode, struct ext4_xattr_info *i,
> bs->s.first = BFIRST(bs->bh);
> bs->s.end = bs->bh->b_data + bs->bh->b_size;
> bs->s.here = bs->s.first;
> - error = xattr_find_entry(inode, &bs->s.here, bs->s.end,
> + error = ext4_xattr_find_entry(inode, &bs->s.here, bs->s.end,
> i->name_index, i->name, 1);
> if (error && error != -ENODATA)
> return error;
> @@ -2222,11 +2222,11 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
> is->s.here = is->s.first;
> is->s.end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
> - error = xattr_check_inode(inode, header, is->s.end);
> + error = ext4_xattr_check_inode(inode, header, is->s.end);
> if (error)
> return error;
> /* Find the named attribute. */
> - error = xattr_find_entry(inode, &is->s.here, is->s.end,
> + error = ext4_xattr_find_entry(inode, &is->s.here, is->s.end,
> i->name_index, i->name, 0);
> if (error && error != -ENODATA)
> return error;
The indentation of arguments in the above should be updated as well to look
like:
error = ext4_xattr_find_entry(inode, &is->s.here, is->s.end,
i->name_index, i->name, 0);
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Tue 06-12-22 09:58:06, Ye Bin wrote:
> From: Ye Bin <[email protected]>
>
> There is issue as follows when do setxattr with inject fault:
> [localhost]#fsck.ext4 -fn /dev/sda
> e2fsck 1.46.6-rc1 (12-Sep-2022)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Unattached zero-length inode 15. Clear? no
>
> Unattached inode 15
> Connect to /lost+found? no
>
> Pass 5: Checking group summary information
>
> /dev/sda: ********** WARNING: Filesystem still has errors **********
>
> /dev/sda: 15/655360 files (0.0% non-contiguous), 66755/2621440 blocks
>
> Above issue occur in 'ext4_xattr_inode_create()', if 'ext4_mark_inode_dirty()'
> failed need to drop inode's i_nlink. Or will lead to inode leak.
>
> Signed-off-by: Ye Bin <[email protected]>
The change looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
I'm just slightly wondering if there is ever a situation when
ext4_mark_inode_dirty() fails but the following cleanup would succeed...
Honza
> ---
> fs/ext4/xattr.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 003fe1f2d6a8..734f787ae7ed 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1464,6 +1464,9 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
> if (!err)
> err = ext4_inode_attach_jinode(ea_inode);
> if (err) {
> + if (ext4_xattr_inode_dec_ref(handle, ea_inode))
> + ext4_warning_inode(ea_inode,
> + "cleanup dec ref error %d", err);
> iput(ea_inode);
> return ERR_PTR(err);
> }
> --
> 2.31.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR