linux-kernel - Re: [PATCH -next 6/6] ext4: fix inode leak in 'ext4_xattr_inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221206121053.ksylcq5ag6yn6yct@quack3>
Date:   Tue, 6 Dec 2022 13:10:53 +0100
From:   Jan Kara <jack@...e.cz>
To:     Ye Bin <yebin@...weicloud.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        jack@...e.cz, Ye Bin <yebin10@...wei.com>
Subject: Re: [PATCH -next 6/6] ext4: fix inode leak in
 'ext4_xattr_inode_create()'

On Tue 06-12-22 09:58:06, Ye Bin wrote:
> From: Ye Bin <yebin10@...wei.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@...wei.com>

The change looks good. Feel free to add:

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

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 <jack@...e.com>
SUSE Labs, CR