Re: [PATCH] isofs: fix out-of-bounds page array access on empty zisofs block

[Jan Kara]

On Sun 12-07-26 16:41:50, Xiang Mei wrote:
> zisofs_uncompress_block()'s empty-block fast path returns
> pcount << PAGE_SHIFT, ignoring the incoming poffset, unlike the
> decompression path which returns bytes produced relative to poffset.
> zisofs_fill_pages() uses that return to advance its page cursor, so when
> the zisofs block size is below PAGE_SIZE and a sub-page block leaves
> poffset partway into a page, a following empty block over-counts and
> advances pages[] one element past its end, after which
> "if (poffset && *pages)" reads pages[1] out of bounds.  rock.c only
> rejects a block-size shift > 17, so a crafted "ZF" Rock Ridge record can
> set it below PAGE_SHIFT; the bug is reached by an ordinary read() of a
> compressed file on such a mounted ISO9660 image.
> 
> Return the byte count relative to poffset and zero only
> [poffset, PAGE_SIZE) of the first page, matching the decompression path.
> The page-aligned case (poffset == 0) is unaffected.
> 
>   BUG: KASAN: slab-out-of-bounds in zisofs_read_folio (fs/isofs/compress.c:290)
>   Read of size 8 at addr ffff88800f5eac48 by task exploit/142
>    zisofs_read_folio (fs/isofs/compress.c:290)
>    read_pages (mm/readahead.c:184)
>    ...
>    filemap_read (mm/filemap.c:2814)
>    vfs_read (fs/read_write.c:574)
>    __x64_sys_pread64 (fs/read_write.c:769)
>    do_syscall_64 (arch/x86/entry/syscall_64.c:94)
>    entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>   The buggy address is located 0 bytes to the right of the
>   allocated 8-byte region in the kmalloc-8 cache
> 
> Fixes: 59bc055211b8 ("zisofs: Implement reading of compressed files when PAGE_CACHE_SIZE > compress block size")
> Reported-by: Weiming Shi <bestswngs@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@xxxxxxx>

Thanks! I've added the patch to my tree.

									Honza

> ---
>  fs/isofs/compress.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/isofs/compress.c b/fs/isofs/compress.c
> index 397568b9c7e7..3fda92358e22 100644
> --- a/fs/isofs/compress.c
> +++ b/fs/isofs/compress.c
> @@ -65,12 +65,14 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
>  	/* Empty block? */
>  	if (block_size == 0) {
>  		for ( i = 0 ; i < pcount ; i++ ) {
> +			unsigned int off = i ? 0 : poffset;
> +
>  			if (!pages[i])
>  				continue;
> -			memzero_page(pages[i], 0, PAGE_SIZE);
> +			memzero_page(pages[i], off, PAGE_SIZE - off);
>  			SetPageUptodate(pages[i]);
>  		}
> -		return ((loff_t)pcount) << PAGE_SHIFT;
> +		return (((loff_t)pcount) << PAGE_SHIFT) - poffset;
>  	}
>  
>  	/* Because zlib is not thread-safe, do all the I/O at the top. */
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR