Ernesto A. Fernándezunread,
Aug 12, 2020, 4:24:28 PM8/12/20
to Dan Carpenter, Peilin Ye, Greg Kroah-Hartman, linux-...@vger.kernel.org, linux-kern...@lists.linuxfoundation.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org
On Wed, Aug 12, 2020 at 11:59:04AM +0300, Dan Carpenter wrote:
> Yeah, the patch doesn't work at all. I looked at one call tree and it
> hfs_mdb_get() tries to allocate HFS_SB(sb)->ext_tree.
> HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
> hfs_btree_open() calls page = read_mapping_page(mapping, 0, NULL);
> read_mapping_page() calls mapping->a_ops->readpage() which leads to
> hfs_readpage() which leads to hfs_ext_read_extent() which calls
> res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
> So we need ->ext_tree to be non-NULL before we can set ->ext_tree to be
> non-NULL... :/
For HFS+, the first 8 extents for a file are kept inside its own fork data
structure, not in the extent tree. So, in normal operation, you don't need
to search the extent tree to find the first page of the extent tree itself.
The HFS layout is different, but it should work the same way.
Of course this sort of thing can still be triggered by crafted filesystems.
If that's what the reproducer is about, I think just returning an error is
reasonable. But these modules will never be safe against attacks such as