From 0ed3734e5e34ea6c47ece2ef2c7c8a0fe0fcf325 Mon Sep 17 00:00:00 2001 From: Michael Giacomelli Date: Sat, 19 Sep 2026 17:48:20 -0400 Subject: [PATCH] flac: cope with frames larger than one buffer request The frame decoder reads from one flat buffer and cannot refill it, but request_buffer() only guarantees 32KiB of contiguous data (the buffering guard area). Frames that can be larger than that, such as high resolution or poorly compressible streams (FLAC decoder testbench file 31), could be handed to the decoder truncated, which read past the end of the data and lost sync. When a request returns less than the largest frame the stream can contain (STREAMINFO max framesize, or a bound from block size, channels and bit depth) and it is not the end of the file, copy the frame into a private static buffer and decode from that. Streams whose frames always fit never touch the buffer and pay one comparison per frame. The buffer is 64KiB, or sized for the 4608 sample blocks of memory limited targets, and is left out entirely when MEMORYSIZE is 2MB or less. Also fail with a codec error, instead of advancing past the data, if a decoded frame consumed more bytes than were provided. Co-Authored-By: Claude Sonnet 5 Change-Id: Ib4cf85ad513f8e096f73d13b4374d072e1745cb2 --- lib/rbcodec/codecs/flac.c | 62 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/lib/rbcodec/codecs/flac.c b/lib/rbcodec/codecs/flac.c index dc3c4ba06f..c2fd70c896 100644 --- a/lib/rbcodec/codecs/flac.c +++ b/lib/rbcodec/codecs/flac.c @@ -75,6 +75,22 @@ static struct FLACseekpoints seekpoints[MAX_SUPPORTED_SEEKTABLE_SIZE]; static int nseekpoints; static int8_t *bit_buffer; +/* Frame copy buffer for flac_request_frame(). Targets with 2MB of RAM or + less (e.g. the Clip v1) cannot spare it and rely on the overrun check in + codec_run() alone. */ +#if MEMORYSIZE > 2 +#define FLAC_FRAME_COPY +#if MAX_BLOCKSIZE > 4608 +#define FRAME_COPY_SIZE MAX_FRAMESIZE +#else +/* Blocks are capped at 4608 samples here (see bitstream.h): size for the + largest stereo frame, 24 bit with verbatim subframes, plus headers. */ +#define FRAME_COPY_SIZE (MAX_BLOCKSIZE * 2 * 3 + 64) +#endif +/* Largest frame the current stream can contain */ +static size_t frame_bound; +static uint8_t frame_copy[FRAME_COPY_SIZE]; +#endif static size_t buff_size; static bool flac_init(FLACContext* fc, int first_frame_offset) @@ -209,6 +225,17 @@ static bool flac_init(FLACContext* fc, int first_frame_offset) } if (found_streaminfo) { +#ifdef FLAC_FRAME_COPY + frame_bound = fc->max_framesize; + if (frame_bound == 0) { + /* stream did not encode max frame size, assume worst case of + verbatim subframes plus headers */ + frame_bound = ((size_t)fc->max_blocksize * fc->channels * fc->bps + + 7) / 8 + 64; + } + if (frame_bound > FRAME_COPY_SIZE) + frame_bound = FRAME_COPY_SIZE; +#endif /* length is 0 when STREAMINFO has no total sample count */ fc->bitrate = fc->length ? ((int64_t) (fc->filesize-fc->metadatalength) * 8) / fc->length : 0; @@ -218,6 +245,29 @@ static bool flac_init(FLACContext* fc, int first_frame_offset) } } +/* request_buffer() only guarantees 32KiB of contiguous data, but a FLAC frame + can be larger, and the frame decoder cannot refill its buffer mid-frame. + When a request returns less than frame_bound before the end of the file, + the frame is copied into frame_copy and decoded from there. Frames + normally fit, so the usual cost is one comparison per frame. */ +#ifdef FLAC_FRAME_COPY +static void *flac_request_frame(size_t *len, size_t reqsize) +{ + void *buf = ci->request_buffer(len, reqsize); + + if (*len >= frame_bound || *len >= (size_t)(ci->filesize - ci->curpos)) + return buf; + + off_t pos = ci->curpos; + size_t got = ci->read_filebuf(frame_copy, frame_bound); + ci->seek_buffer(pos); + *len = got; + return frame_copy; +} +#else +#define flac_request_frame(len, reqsize) ci->request_buffer(len, reqsize) +#endif + /* Synchronize to next frame in stream - adapted from libFLAC 1.1.3b2 */ static bool frame_sync(FLACContext* fc) { unsigned int x = 0; @@ -256,7 +306,7 @@ static bool frame_sync(FLACContext* fc) { /* Advance and init bit buffer to the new frame. */ ci->advance_buffer((get_bits_count(&fc->gb)-16)>>3); /* consumed bytes */ - bit_buffer = ci->request_buffer(&buff_size, MAX_FRAMESIZE+16); + bit_buffer = flac_request_frame(&buff_size, MAX_FRAMESIZE+16); init_get_bits(&fc->gb, bit_buffer, buff_size*8); /* Decode the frame to verify the frame crc and @@ -503,7 +553,7 @@ enum codec_status codec_run(void) ci->set_elapsed(elapsedtime); /* The main decoding loop */ - buf = ci->request_buffer(&bytesleft, MAX_FRAMESIZE); + buf = flac_request_frame(&bytesleft, MAX_FRAMESIZE); while (bytesleft) { long action = ci->get_command(¶m); @@ -515,7 +565,7 @@ enum codec_status codec_run(void) if (flac_seek(&fc,(uint32_t)(((uint64_t)param *ci->id3->frequency)/1000))) { /* Refill the input buffer */ - buf = ci->request_buffer(&bytesleft, MAX_FRAMESIZE); + buf = flac_request_frame(&bytesleft, MAX_FRAMESIZE); } ci->set_elapsed(param); @@ -528,6 +578,10 @@ enum codec_status codec_run(void) return CODEC_ERROR; } consumed=fc.gb.index/8; + if (res == 0 && consumed > (int)bytesleft) { + LOGF("FLAC: Frame overran its %d bytes of data\n", (int)bytesleft); + return CODEC_ERROR; + } #if defined(LOGF_ENABLE) frame++; #endif @@ -545,7 +599,7 @@ enum codec_status codec_run(void) ci->advance_buffer(consumed); - buf = ci->request_buffer(&bytesleft, MAX_FRAMESIZE); + buf = flac_request_frame(&bytesleft, MAX_FRAMESIZE); } LOGF("FLAC: Decoded %lu samples\n",(unsigned long)samplesdone);