mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 17:28:49 -10:00
rpng: keep draining inflate after the compressed input runs out
Regression frombbf299b: images whose scanline is wider than the 32 KB inflate slice decode as black. A 16384-wide RGB8 image has a 49153-byte scanline and reproduces it; so does 16384-wide RGBA8. The driver stopped calling the backend as soon as avail_in hit zero: bool to_continue = (process->avail_in > 0 && process->avail_out > 0); An inflate backend pulls whole input bytes into its bit accumulator ahead of use and reports them as consumed, so "input consumed" has never meant "output finished" - there can be an accumulator's worth of decodable data left after the last byte has been handed over. The condition above threw those trailing bytes away, and the under-produced check at the end of the same function then failed the whole image, which surfaces as a black texture. This was always latent - zlib holds up to 4 bytes the same way - butbbf299bwidened the clean-room backend's accumulator from 32 to 64 bits, so it now reads up to 8 bytes ahead and the last call routinely ends owing output with no input left. On the failing images it stopped exactly 2 bytes short of the declared geometry: [c] 2046 slice=32768 rd=19392 wn=32768 avail_in=19392 avail_out=32770 under-produced: avail_in=0 avail_out=2 total_out=50332670 Drive on output instead, which is the condition that actually describes the work left, and take the termination guarantee that avail_in > 0 was silently providing from an explicit no-progress check: a call that neither consumes input nor produces output cannot get any further, so the stream is truncated or corrupt. When every IDAT span has been consumed the backend now gets an explicit empty input window rather than being left pointing at the previous one. The 32-bit-accumulator backends were reachable by the same path with a smaller margin, so this hardens HAVE_ZLIB=1 builds too. Validation, under -fsanitize=address,undefined, for HAVE_ZLIB=1 and HAVE_ZLIB=0 builds: - 1656 images byte-exact against libpng 1.6.43, covering every colour type, bit depth 1/2/4/8/16, all five filters, Adam7, widths 1-20 and the edge widths, plus four 16384-wide images (RGB8 and RGBA8, Sub and Paeth) that fail without this patch. - 681 truncated and bit-flipped PNGs: all rejected by both backends, no hangs, no sanitizer reports. The no-progress check is what keeps a truncated stream from spinning now that avail_in no longer bounds the loop. C89 verified on x86-64 and aarch64.
This commit is contained in:
1 parent
bbf299b3b8
commit
551ff75082
1 file changed
+25
-2
@@ -1795,8 +1795,16 @@ static int rpng_load_image_argb_process_inflate_init(
|
||||
enum trans_stream_error err;
|
||||
uint32_t rd, wn, slice;
|
||||
struct rpng_process *process = (struct rpng_process*)rpng->process;
|
||||
bool to_continue = (process->avail_in > 0
|
||||
&& process->avail_out > 0);
|
||||
/* Keep going while output is still owed, even once the compressed
|
||||
* input is exhausted. An inflate backend buffers whole input bytes
|
||||
* into its bit accumulator ahead of use and reports them as read, so
|
||||
* "input consumed" does not mean "output finished" - there can be a
|
||||
* bit accumulator's worth of decodable data left after the last byte
|
||||
* has been handed over. Requiring avail_in > 0 here truncated those
|
||||
* trailing bytes and then failed the whole image on the
|
||||
* under-produced check below. Progress is enforced after the call
|
||||
* instead. */
|
||||
bool to_continue = (process->avail_out > 0);
|
||||
|
||||
if (!to_continue)
|
||||
goto end;
|
||||
@@ -1822,6 +1830,14 @@ static int rpng_load_image_argb_process_inflate_init(
|
||||
rpng->buff_start + sp->off + process->span_pos,
|
||||
sp->len - process->span_pos);
|
||||
}
|
||||
else
|
||||
{
|
||||
/* All spans consumed: declare an empty input window so the
|
||||
* backend drains whatever it still holds internally rather than
|
||||
* re-reading the previous one. */
|
||||
process->stream_backend->set_in(process->stream,
|
||||
rpng->buff_start, 0);
|
||||
}
|
||||
|
||||
{
|
||||
size_t wpos = process->inflated_total % process->ring_size;
|
||||
@@ -1860,6 +1876,13 @@ static int rpng_load_image_argb_process_inflate_init(
|
||||
process->total_out += wn;
|
||||
process->inflated_total += wn;
|
||||
|
||||
/* No input taken and no output made means the backend cannot get any
|
||||
* further - a truncated or corrupt stream. Without this the
|
||||
* avail_in > 0 condition above was doing double duty as the
|
||||
* termination guarantee. */
|
||||
if (rd == 0 && wn == 0)
|
||||
goto error;
|
||||
|
||||
if (err)
|
||||
return 0;
|
||||
|
||||
|
||||
Reference in new issue
Block a user