linux-kernel - Re: [PATCH] f2fs: Fix slab-out-of-bounds Read KASAN bug 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] [day] [month] [year] [list]

Message-ID: <60ec8f56-5d00-426a-978d-a31894262705@kernel.org>
Date: Wed, 8 Jan 2025 19:44:03 +0800
From: Chao Yu <chao@...nel.org>
To: qasdev <qasdev00@...il.com>, Jaegeuk Kim <jaegeuk@...nel.org>
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] f2fs: Fix slab-out-of-bounds Read KASAN bug in
 f2fs_getxattr()

Hi Qasim,

On 2025/1/8 07:03, qasdev wrote:
> In f2fs_getxattr(), the function lookup_all_xattrs() allocates a 12-byte
> (base_size) buffer for an inline extended attribute. However, when
> __find_inline_xattr() calls __find_xattr(), it uses the macro
> "list_for_each_xattr(entry, addr)", which starts by calling
> XATTR_FIRST_ENTRY(addr). This skips a 24-byte struct f2fs_xattr_header
> at the beginning of the buffer, causing an immediate out-of-bounds read
> in a 12-byte allocation. The subsequent !IS_XATTR_LAST_ENTRY(entry)
> check then dereferences memory outside the allocated region, triggering
> the slab-out-of bounds read.
> 
> This patch prevents the out-of-bounds read by adding a check to bail
> out early if inline_size is too small and does not account for the
> header plus the 4-byte value that IS_XATTR_LAST_ENTRY reads.

Thank you very much for analyzing this issue, the root cause you figured 
out makes sense to me.

Can you please check the patch in below link? It seems it can fix this 
issue as well? IIUC.

https://lore.kernel.org/linux-f2fs-devel/20241216134600.8308-1-chao@kernel.org/

Thanks,

> 
> Reported-by: syzbot <syzbot+f5e74075e096e757bdbf@...kaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=f5e74075e096e757bdbf
> Tested-by: syzbot <syzbot+f5e74075e096e757bdbf@...kaller.appspotmail.com>
> Tested-by: Qasim Ijaz <qasdev00@...il.com>
> Fixes: 388a2a0640e1 ("f2fs: remove redundant sanity check in sanity_check_inode()")
> Signed-off-by: Qasim Ijaz <qasdev00@...il.com>
> ---
>   fs/f2fs/xattr.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index 3f3874943679..cf82646bca0e 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -329,6 +329,9 @@ static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
>   	if (!xnid && !inline_size)
>   		return -ENODATA;
>   
> +	if (inline_size < sizeof(struct f2fs_xattr_header) + sizeof(__u32))
> +		return -ENODATA;
> +
>   	*base_size = XATTR_SIZE(inode) + XATTR_PADDING_SIZE;
>   	txattr_addr = xattr_alloc(F2FS_I_SB(inode), *base_size, is_inline);
>   	if (!txattr_addr)