linux-kernel - Re: [PATCH] hfs: Fix OOB Read in __hfs_brec

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: <AF1D5323-061B-4B7E-83E4-90BF1275DB30@dubeyko.com>
Date:   Wed, 30 Nov 2022 11:23:56 -0800
From:   Viacheslav Dubeyko <slava@...eyko.com>
To:     Peng Zhang <zhangpeng362@...wei.com>
Cc:     "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        Jeff Layton <jlayton@...nel.org>,
        Ira Weiny <ira.weiny@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, sunnanyong@...wei.com,
        wangkefeng.wang@...wei.com,
        syzbot+e836ff7133ac02be825f@...kaller.appspotmail.com
Subject: Re: [PATCH] hfs: Fix OOB Read in __hfs_brec_find



> On Nov 29, 2022, at 10:59 PM, Peng Zhang <zhangpeng362@...wei.com> wrote:
> 
> From: ZhangPeng <zhangpeng362@...wei.com>
> 
> Syzbot reported a OOB read bug:
> 
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in hfs_strcmp+0x117/0x190
> fs/hfs/string.c:84
> Read of size 1 at addr ffff88807eb62c4e by task kworker/u4:1/11
> CPU: 1 PID: 11 Comm: kworker/u4:1 Not tainted
> 6.1.0-rc6-syzkaller-00308-g644e9524388a #0
> Workqueue: writeback wb_workfn (flush-7:0)
> 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_strcmp+0x117/0x190 fs/hfs/string.c:84
> __hfs_brec_find+0x213/0x5c0 fs/hfs/bfind.c:75
> hfs_brec_find+0x276/0x520 fs/hfs/bfind.c:138
> hfs_write_inode+0x34c/0xb40 fs/hfs/inode.c:462
> write_inode fs/fs-writeback.c:1440 [inline]
> 
> If the input inode of hfs_write_inode() is incorrect:
> struct inode
>  struct hfs_inode_info
>    struct hfs_cat_key
>      struct hfs_name
>        u8 len # len is greater than HFS_NAMELEN(31) which is the
> maximum length of an HFS filename
> 
> OOB read occurred:
> hfs_write_inode()
>  hfs_brec_find()
>    __hfs_brec_find()
>      hfs_cat_keycmp()
>        hfs_strcmp() # OOB read occurred due to len is too large
> 
> Fix this by adding a Check on len in hfs_write_inode() before calling
> hfs_brec_find().
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+e836ff7133ac02be825f@...kaller.appspotmail.com
> Signed-off-by: ZhangPeng <zhangpeng362@...wei.com>
> ---
> fs/hfs/inode.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index c4526f16355d..a0746be3c1de 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -458,6 +458,8 @@ int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
> 		/* panic? */
> 		return -EIO;
> 
> +	if (HFS_I(main_inode)->cat_key.CName.len > HFS_NAMELEN)
> +		return -EIO;

If I understood correctly, we have corrupted struct hfs_cat_key instance. But what is the initial place
of this corruption? What function could introduce such corruption? Maybe, it needs to find a place(s)
where we can add some additional check and potentially exclude the incorrect input into
hfs_write_inode()?

I think it is not only place where it makes sense to check the correctness of struct hfs_cat_key
instance. Could we introduce a special function that check struct hfs_cat_key on corrupted
state and to use this function to check the state of the key in functions that operates by
keys?

Thanks,
Slava. 

> 	fd.search_key->cat = HFS_I(main_inode)->cat_key;
> 	if (hfs_brec_find(&fd))
> 		/* panic? */
> -- 
> 2.25.1
>