Skip to content

decoder/mad: bound Xing seek table allocations - #2555

Open
acts-1631 wants to merge 2 commits into
MusicPlayerDaemon:v0.24.xfrom
acts-1631:fix/mad-xing-frame-count
Open

decoder/mad: bound Xing seek table allocations#2555
acts-1631 wants to merge 2 commits into
MusicPlayerDaemon:v0.24.xfrom
acts-1631:fix/mad-xing-frame-count

Conversation

@acts-1631

Copy link
Copy Markdown
Contributor

DecodeFirstFrame() uses the frame count from a Xing header as
max_frames. RunDecoder() then calls AllocateBuffers(), which eagerly
allocates frame_offsets and times arrays using that count.

The existing limit accepts 8,388,608 frames. On x86_64, the two arrays
request about 192 MiB. A crafted 1,189-byte MPEG file can supply this
count, and allocation failure terminates MPD because AllocateBuffers()
is noexcept.

Ignore Xing frame counts that cannot fit within the input even if every
frame were only a four-byte MPEG header. Unknown-size streams retain
the conservative estimate from FileSizeToSongLength(). Also use
nothrow allocation and clean up partial allocations so a remaining
failure stops this decoder instance instead of terminating MPD.

@MaxKellermann

Copy link
Copy Markdown
Member

This commit mixes several distinct changes that should be separate commits.

The mad decoder uses the Xing frame count to size its seek tables. A
small input can claim millions of frames and request a disproportionate
amount of memory.

Ignore a frame count which cannot fit in the known input, even if each
frame contains only a four-byte MPEG header. For an unknown-size stream,
retain the conservative estimate from FileSizeToSongLength().
The mad decoder's seek tables are allocated from a value derived from
the input. AllocateBuffers() is noexcept, so a failed allocation would
terminate MPD.

Use nothrow allocation and return failure to RunDecoder(). Free the
first table if allocating the second fails.
@acts-1631
acts-1631 force-pushed the fix/mad-xing-frame-count branch from a1fe0ad to 4a2c495 Compare August 2, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants