> [Bug]
> path_openat() called open_last_lookups() before calling do_open() and
> open_last_lookups() will eventually call squashfs_read_inode() to set
> inode->i_size, but before setting i_size, it is necessary to obtain file_size
> from the disk.
>
> However, during the value retrieval process, the length of the value retrieved
> from the disk was greater than output->length, resulting(-EIO) in the failure of
> squashfs_read_data(), further leading to i_size has not been initialized,
> i.e. its value is 0.
>
NACK
This analysis is completely *wrong*. First, if there was I/O error reading
the inode it would never be created, and squasfs_readahead() would
never be called on it, because it will never exist.
Second i_size isn't unintialised and it isn't 0 in value. Where
you got this bogus information from is because in your test patches,
i.e.
https://lore.kernel.org/all/[email protected]/
You have
+ if (!file_end) {
+ printk("i:%p, is:%d, %s\n", inode, i_size_read(inode), __func__);
+ res = -EINVAL;
+ goto out;
+ }
+
You have used %d, and the result of i_size_read(inode) overflows, giving the
bogus 0 value.
The actual value is 1407374883553280, or 0x5000000000000, which is
too big to fit into an unsigned int.
> This resulted in the failure of squashfs_read_data(), where "SQUASHFS error:
> Failed to read block 0x6fc: -5" was output in the syz log.
> This also resulted in the failure of squashfs_cache_get(), outputting "SQUASHFS
> error: Unable to read metadata cache entry [6fa]" in the syz log.
>
NO, *that* is caused by the failure to read some other inodes which
as a result are correctly not created. Nothing to do with the oops here.
> [Fix]
> Before performing a read ahead operation in squashfs_read_folio() and
> squashfs_readahead(), check if i_size is not 0 before continuing.
>
A third NO, it is only 0 because the variable overflowed.
Additionally, let's look at your "fix" here.
> @@ -461,6 +461,11 @@ static int squashfs_read_folio(struct file *file, struct folio *folio)
> TRACE("Entered squashfs_readpage, page index %lx, start block %llx\n",
> page->index, squashfs_i(inode)->start);
>
> + if (!file_end) {
> + res = -EINVAL;
> + goto out;
> + }
> +
file_end is computed by
int file_end = i_size_read(inode) >> msblk->block_log;
So your "fix" will reject *any* file less than msblk->block_log in
size as invalid, including perfectly valid zero size files (empty
files are valid too).
I already identified the cause and send a fix patch here:
https://lore.kernel.org/all/[email protected]/
NACK
Phillip