> On Nov 29, 2022, at 10:59 PM, Peng Zhang <zhangpeng362@xxxxxxxxxx> wrote:
>
> From: ZhangPeng <zhangpeng362@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: ZhangPeng <zhangpeng362@xxxxxxxxxx>
> ---
> 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
>
