Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround crash in atrac3+ decoding #66

Closed
wants to merge 3 commits into from
Closed

Workaround crash in atrac3+ decoding #66

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6f9c78
Update the ARM64 android build script
hrydgard Sep 8, 2022
0e9e153
Give the decoder more memory to read from to workaround bad bit readi…
hrydgard Sep 8, 2022
5f93a31
Update ARM64 binaries
hrydgard Sep 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions libavcodec/atrac3plusdec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,24 +339,38 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,
if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
return ret;

if ((ret = init_get_bits8(&ctx->gb, avpkt->data, avpkt->size)) < 0)
// PPSSPP workaround: With bad/corrupt input, the atrac3plus decoder does not
// reliably stay inside the bounds of the buffer. Instead of carefully checking everything
// inside it, for now let's just give it more space to read from.
const int extra_bytes = 1024;

uint8_t *bigger_buffer = malloc(avpkt->size + extra_bytes);
memset(bigger_buffer + avpkt->size, 0, extra_bytes);
memcpy(bigger_buffer, avpkt->data, avpkt->size);
Comment on lines -342 to +349
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this directly the packet buffer we pass in via avcodec_decode_audio4? If so, we can control the buffer size there. Usually it points at PSP RAM, though... but it can also point to dataBuf_. Does padding that out help?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was so happy to have found the workaround that I forgot to track outwards to see if we control the buffer. Will have a look later today.


if ((ret = init_get_bits8(&ctx->gb, bigger_buffer, avpkt->size)) < 0) {
free(bigger_buffer);
return ret;
}

if (get_bits1(&ctx->gb)) {
av_log(avctx, AV_LOG_ERROR, "Invalid start bit!\n");
free(bigger_buffer);
return AVERROR_INVALIDDATA;
}

while (get_bits_left(&ctx->gb) >= 2 &&
(ch_unit_id = get_bits(&ctx->gb, 2)) != CH_UNIT_TERMINATOR) {
if (ch_unit_id == CH_UNIT_EXTENSION) {
avpriv_report_missing_feature(avctx, "Channel unit extension");
free(bigger_buffer);
return AVERROR_PATCHWELCOME;
}
if (ch_block >= ctx->num_channel_blocks ||
ctx->channel_blocks[ch_block] != ch_unit_id) {
av_log(avctx, AV_LOG_ERROR,
"Frame data doesn't match channel configuration!\n");
free(bigger_buffer);
return AVERROR_INVALIDDATA;
}

Expand All @@ -366,8 +380,10 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,
if ((ret = ff_atrac3p_decode_channel_unit(&ctx->gb,
&ctx->ch_units[ch_block],
channels_to_process,
avctx)) < 0)
avctx)) < 0) {
free(bigger_buffer);
return ret;
}

decode_residual_spectrum(&ctx->ch_units[ch_block], ctx->samples,
channels_to_process, avctx);
Expand All @@ -384,6 +400,7 @@ static int atrac3p_decode_frame(AVCodecContext *avctx, void *data,

*got_frame_ptr = 1;

free(bigger_buffer);
return FFMIN(avctx->block_align, avpkt->size);
}

Expand Down