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: <08098e46-0468-4fec-b2fb-9ea7414eaea0@kernel.org>
Date: Thu, 9 Jan 2025 17:02:02 +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()

On 1/9/25 00:23, qasdev wrote:
> On Wed, Jan 08, 2025 at 07:44:03PM +0800, Chao Yu wrote:
>> 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,
> 
> Hi Chao,
> 
> I tested the patch you linked on my machine and with syzbot, and both tests succeeded. The patch you linked works very well.

Hi Qasdev,

Thanks for the test!

>   
> Here is the link to the results of the testing of both patches: https://syzkaller.appspot.com/bug?extid=f5e74075e096e757bdbf
> 
> Would it be possible to include me in the Tested-by header and any other contribution acknowledgments you feel appropriate?
 > > Thanks!
> 
> Best regards,
> Qasim
> 
>>
>>>
>>> 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>

IMO, it will be better to quoted your comment description and all above 
tags into the patch, what do you think?

Thanks,

>>> 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)
>>