search

AgentD / squashfs-tools-ng

A new set of tools and libraries for working with SquashFS images
Other
194 stars 30 forks source link

gensquashfs hangs up when reading oversized packfiles #121

Closed jwise closed 5 months ago

jwise commented 8 months ago

Unfortunately, I don't have time to come up with a repro case that does not contain proprietary data, but I just did a git bisect and found that commit c7eae87870da755586446fecb4eb30063c1d641c introduces a regression. One packfile, but not all packfiles, seems to result in gensquashfs hanging while reading the packfile; breaking in with GDB a few times and inspecting register state shows that it is hung in the loop in istream_get_line, with count = 0. It seems to go wrong, suspiciously, at offset 0x20000 into the packfile... so maybe the issue is that all oversized packfiles will break?

AgentD commented 8 months ago

Hi @jwise ,

you are presumably working on current master? I think I managed to reproduce the problem: 0x20000 (=128k) is the size of the internal buffer in the file stream. This was introduced by a really dumb mistake in the buffer accounting that was introduced by the commit you mentioned, that restructures the file-stream buffer logic.

The change moved the responsibility of the buffer into the implementation. Two callbacks are used to process data: one to get a pointer into the buffer and the available byte count, another to mark data as consumed. The first one also does the refill if the buffer is empty.

The overall idea was to reduce the amount of buffer-to-buffer copying. A SquashFS based stream can provide data directly from the decompression buffer from which we write it directly out into a tarball or a tar based file stream can directly provide data from the buffer of the underlying file stream, which we can hand over to the SquashFS packer.

The issue with get_line arises when the line-break exactly aligns with the end of the buffer. We end up with offset==available, both non-zero, so the refill does not trigger.

Commit 2eec954d10396fba1755261f3d911701818755d8 should fix the issue (at least my quick test setup could not trigger it anymore). Both counters are now reset to 0 when everything was consumed, and the buffer-fetch callback triggers the refill.

I hope that fixes it, thanks for reporting this!