linux-kernel - Re: [PATCH 5.10 60/76] f2fs: fix to do sanity check on last xattr entry in __f2fs

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]

Date:   Mon, 3 Jan 2022 22:11:18 +0100
From:   Salvatore Bonaccorso <carnil@...ian.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Wenqing Liu <wenqingliu0120@...il.com>,
        Chao Yu <chao@...nel.org>, Jaegeuk Kim <jaegeuk@...nel.org>
Subject: Re: [PATCH 5.10 60/76] f2fs: fix to do sanity check on last xattr
 entry in __f2fs_setxattr()

Hi,

On Mon, Dec 27, 2021 at 04:31:15PM +0100, Greg Kroah-Hartman wrote:
> From: Chao Yu <chao@...nel.org>
> 
> commit 5598b24efaf4892741c798b425d543e4bed357a1 upstream.
> 
> As Wenqing Liu reported in bugzilla:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=215235
> 
> - Overview
> page fault in f2fs_setxattr() when mount and operate on corrupted image
> 
> - Reproduce
> tested on kernel 5.16-rc3, 5.15.X under root
> 
> 1. unzip tmp7.zip
> 2. ./single.sh f2fs 7
> 
> Sometimes need to run the script several times
> 
> - Kernel dump
> loop0: detected capacity change from 0 to 131072
> F2FS-fs (loop0): Found nat_bits in checkpoint
> F2FS-fs (loop0): Mounted with checkpoint version = 7548c2ee
> BUG: unable to handle page fault for address: ffffe47bc7123f48
> RIP: 0010:kfree+0x66/0x320
> Call Trace:
>  __f2fs_setxattr+0x2aa/0xc00 [f2fs]
>  f2fs_setxattr+0xfa/0x480 [f2fs]
>  __f2fs_set_acl+0x19b/0x330 [f2fs]
>  __vfs_removexattr+0x52/0x70
>  __vfs_removexattr_locked+0xb1/0x140
>  vfs_removexattr+0x56/0x100
>  removexattr+0x57/0x80
>  path_removexattr+0xa3/0xc0
>  __x64_sys_removexattr+0x17/0x20
>  do_syscall_64+0x37/0xb0
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> The root cause is in __f2fs_setxattr(), we missed to do sanity check on
> last xattr entry, result in out-of-bound memory access during updating
> inconsistent xattr data of target inode.
> 
> After the fix, it can detect such xattr inconsistency as below:
> 
> F2FS-fs (loop11): inode (7) has invalid last xattr entry, entry_size: 60676
> F2FS-fs (loop11): inode (8) has corrupted xattr
> F2FS-fs (loop11): inode (8) has corrupted xattr
> F2FS-fs (loop11): inode (8) has invalid last xattr entry, entry_size: 47736
> 
> Cc: stable@...r.kernel.org
> Reported-by: Wenqing Liu <wenqingliu0120@...il.com>
> Signed-off-by: Chao Yu <chao@...nel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@...nel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  fs/f2fs/xattr.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -680,8 +680,17 @@ static int __f2fs_setxattr(struct inode
>  	}
>  
>  	last = here;
> -	while (!IS_XATTR_LAST_ENTRY(last))
> +	while (!IS_XATTR_LAST_ENTRY(last)) {
> +		if ((void *)(last) + sizeof(__u32) > last_base_addr ||
> +			(void *)XATTR_NEXT_ENTRY(last) > last_base_addr) {
> +			f2fs_err(F2FS_I_SB(inode), "inode (%lu) has invalid last xattr entry, entry_size: %zu",
> +					inode->i_ino, ENTRY_SIZE(last));
> +			set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> +			error = -EFSCORRUPTED;
> +			goto exit;
> +		}
>  		last = XATTR_NEXT_ENTRY(last);
> +	}
>  
>  	newsize = XATTR_ALIGN(sizeof(struct f2fs_xattr_entry) + len + size);

It looks this commit while it was applied to several stable series
(TTBOMK in 5.15.12, 5.10.89, 5.4.169, 4.19.223 and 4.14.260) it is
still missing from mainline, Chao, or anyone else, do you know what
happened here?

Regards,
Salvatore