> [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/000000000000bb74b9060a14717c@xxxxxxxxxx/ 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/20231113160901.6444-1-phillip@xxxxxxxxxxxxxxx/ NACK Phillip
