lkml.org 
[lkml]   [2022]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] hfs: Fix OOB Write in hfs_asc2mac
From

On 2022/11/30 3:08, Viacheslav Dubeyko wrote:
>> On Nov 28, 2022, at 6:23 PM, zhangpeng (AS) <zhangpeng362@huawei.com> wrote:
>>
>> On 2022/11/29 3:29, Viacheslav Dubeyko wrote:
>>>> On Nov 25, 2022, at 8:36 PM, Peng Zhang <zhangpeng362@huawei.com> wrote:
>>>>
>>>> From: ZhangPeng <zhangpeng362@huawei.com>
>>>>
>>>> Syzbot reported a OOB Write bug:
>>>>
>>>> loop0: detected capacity change from 0 to 64
>>>> ==================================================================
>>>> BUG: KASAN: slab-out-of-bounds in hfs_asc2mac+0x467/0x9a0
>>>> fs/hfs/trans.c:133
>>>> Write of size 1 at addr ffff88801848314e by task syz-executor391/3632
>>>>
>>>> Call Trace:
>>>> <TASK>
>>>> __dump_stack lib/dump_stack.c:88 [inline]
>>>> dump_stack_lvl+0x1b1/0x28e lib/dump_stack.c:106
>>>> print_address_description+0x74/0x340 mm/kasan/report.c:284
>>>> print_report+0x107/0x1f0 mm/kasan/report.c:395
>>>> kasan_report+0xcd/0x100 mm/kasan/report.c:495
>>>> hfs_asc2mac+0x467/0x9a0 fs/hfs/trans.c:133
>>>> hfs_cat_build_key+0x92/0x170 fs/hfs/catalog.c:28
>>>> hfs_lookup+0x1ab/0x2c0 fs/hfs/dir.c:31
>>>> lookup_open fs/namei.c:3391 [inline]
>>>> open_last_lookups fs/namei.c:3481 [inline]
>>>> path_openat+0x10e6/0x2df0 fs/namei.c:3710
>>>> do_filp_open+0x264/0x4f0 fs/namei.c:3740
>>>>
>>>> If in->len is much larger than HFS_NAMELEN(31) which is the maximum
>>>> length of an HFS filename, a OOB Write could occur in hfs_asc2mac(). In
>>>> that case, when the dst reaches the boundary, the srclen is still
>>>> greater than 0, which causes a OOB Write.
>>>> Fix this by adding a Check on dstlen before Writing to dst address.
>>>>
>>>> Fixes: 328b92278650 ("[PATCH] hfs: NLS support")
>>>> Reported-by: syzbot+dc3b1cf9111ab5fe98e7@syzkaller.appspotmail.com
>>>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>>>> ---
>>>> fs/hfs/trans.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/fs/hfs/trans.c b/fs/hfs/trans.c
>>>> index 39f5e343bf4d..886158db07b3 100644
>>>> --- a/fs/hfs/trans.c
>>>> +++ b/fs/hfs/trans.c
>>>> @@ -130,6 +130,8 @@ void hfs_asc2mac(struct super_block *sb, struct hfs_name *out, const struct qstr
>>>> dst += size;
>>>> dstlen -= size;
>>>> } else {
>>>> + if (dstlen == 0)
>>>> + goto out;
>>> Maybe, it makes sense to use dstlen instead of srclen in while()?
>>>
>>> We have now:
>>>
>>> while (srclen > 0) {
>>> <skipped>
>>> } else {
>>> <skipped>
>>> }
>>>
>>> We can use instead:
>>>
>>> while (dstlen > 0) {
>>> <skipped>
>>> } else {
>>> <skipped>
>>> }
>>>
>>> Will it fix the issue?
>>>
>>> Thanks,
>>> Slava.
>> Thank you for your help.
>>
>> After testing, it fix the issue.
>> Would it be better to add dstlen > 0 instead of replacing srclen > 0 with dstlen > 0?
>> Because there may be dstlen > 0 and srclen <= 0.
>>
>> we can use:
>>
>> while (srclen > 0 && dstlen > 0) {
>> <skipped>
>> } else {
>> <skipped>
>> }
>>
> Looks good to me.

Can I put you down as a Reviewed-by or Suggested-by?

Thanks,
Zhang Peng

> Thanks,
> Slava.
>
>> Thanks,
>> Zhang Peng
>>
>>>> *dst++ = ch > 0xff ? '?' : ch;
>>>> dstlen--;
>>>> }
>>>> --
>>>> 2.25.1

\
 
 \ /
  Last update: 2022-12-01 02:55    [W:0.083 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site