[v2,6/6] ext4: fix inode leak in 'ext4_xattr_inode_create()'

Message ID 20221207074043.1286731-7-yebin@huaweicloud.com
State New
Headers
Series Fix two issue about ext4 extended attribute |

Commit Message

Ye Bin Dec. 7, 2022, 7:40 a.m. UTC
  From: Ye Bin <yebin10@huawei.com>

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 <yebin10@huawei.com>
---
 fs/ext4/xattr.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Bagas Sanjaya Dec. 7, 2022, 7:44 a.m. UTC | #1
On Wed, Dec 07, 2022 at 03:40:43PM +0800, Ye Bin wrote:
> 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.
> 

What about "This occurs in ... . If ... fails, dropping i_nlink of the
inode is needed, otherwise inode leak can occur."?
  
Jan Kara Dec. 7, 2022, 11 a.m. UTC | #2
On Wed 07-12-22 15:40:43, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> 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 <yebin10@huawei.com>

I think I've already given my Reviewed-by on this :). Anyway, the patch
looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/xattr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 5c0476ff62c8..6c19d01ba261 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1465,6 +1465,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
>
  

Patch

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5c0476ff62c8..6c19d01ba261 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1465,6 +1465,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);
 		}