| Message ID | 20230617025442.1621-1-anton@khirnov.net |
|---|---|
| State | Accepted |
| Commit | 593e8c2c6ac0571df036c09bf103b7e58445d75d |
| Headers | show |
| Series | [FFmpeg-devel] lavc/binkaudio: reset input packet on errors | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Sat, Jun 17, 2023 at 4:54 AM Anton Khirnov <anton@khirnov.net> wrote: > Make sure we don't repeatedly try to decode the same packet, making no > progress and possibly causing an infinite loop. > Doesn't all error paths, bellow goto label in function, needs this (reset of ch_offset to 0 and unref of pkt) ? > --- > libavcodec/binkaudio.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/binkaudio.c b/libavcodec/binkaudio.c > index f28ecba760..265f93a822 100644 > --- a/libavcodec/binkaudio.c > +++ b/libavcodec/binkaudio.c > @@ -325,7 +325,7 @@ again: > if (s->ch_offset == 0) { > frame->nb_samples = s->frame_len; > if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) > - return ret; > + goto fail; > if (!new_pkt) > frame->pts = AV_NOPTS_VALUE; > } > @@ -334,8 +334,8 @@ again: > avctx->codec->id == AV_CODEC_ID_BINKAUDIO_DCT, > FFMIN(MAX_CHANNELS, s->channels - s->ch_offset), > s->ch_offset)) { > av_log(avctx, AV_LOG_ERROR, "Incomplete packet\n"); > - s->ch_offset = 0; > - return AVERROR_INVALIDDATA; > + ret = AVERROR_INVALIDDATA; > + goto fail; > } > s->ch_offset += MAX_CHANNELS; > get_bits_align32(gb); > -- > 2.40.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Quoting Paul B Mahol (2023-06-17 08:35:21) > On Sat, Jun 17, 2023 at 4:54 AM Anton Khirnov <anton@khirnov.net> wrote: > > > Make sure we don't repeatedly try to decode the same packet, making no > > progress and possibly causing an infinite loop. > > > > Doesn't all error paths, bellow goto label in function, needs this (reset > of ch_offset to 0 and unref of pkt) ? I see no other error paths in that function, beside handling the failure of ff_decode_get_packet().
On Sat, Jun 17, 2023 at 9:16 AM Anton Khirnov <anton@khirnov.net> wrote: > Quoting Paul B Mahol (2023-06-17 08:35:21) > > On Sat, Jun 17, 2023 at 4:54 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > Make sure we don't repeatedly try to decode the same packet, making no > > > progress and possibly causing an infinite loop. > > > > > > > Doesn't all error paths, bellow goto label in function, needs this (reset > > of ch_offset to 0 and unref of pkt) ? > > I see no other error paths in that function, beside handling the failure > of ff_decode_get_packet(). > > get_buffer? > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Quoting Paul B Mahol (2023-06-17 09:39:24) > On Sat, Jun 17, 2023 at 9:16 AM Anton Khirnov <anton@khirnov.net> wrote: > > > Quoting Paul B Mahol (2023-06-17 08:35:21) > > > On Sat, Jun 17, 2023 at 4:54 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > > > Make sure we don't repeatedly try to decode the same packet, making no > > > > progress and possibly causing an infinite loop. > > > > > > > > > > Doesn't all error paths, bellow goto label in function, needs this (reset > > > of ch_offset to 0 and unref of pkt) ? > > > > I see no other error paths in that function, beside handling the failure > > of ff_decode_get_packet(). > > > > > get_buffer? I am adding goto fail for the get_buffer() call in this patch, so I don't understand what your concern is.
On Sat, Jun 17, 2023 at 9:47 AM Anton Khirnov <anton@khirnov.net> wrote: > Quoting Paul B Mahol (2023-06-17 09:39:24) > > On Sat, Jun 17, 2023 at 9:16 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > Quoting Paul B Mahol (2023-06-17 08:35:21) > > > > On Sat, Jun 17, 2023 at 4:54 AM Anton Khirnov <anton@khirnov.net> > wrote: > > > > > > > > > Make sure we don't repeatedly try to decode the same packet, > making no > > > > > progress and possibly causing an infinite loop. > > > > > > > > > > > > > Doesn't all error paths, bellow goto label in function, needs this > (reset > > > > of ch_offset to 0 and unref of pkt) ? > > > > > > I see no other error paths in that function, beside handling the > failure > > > of ff_decode_get_packet(). > > > > > > > > get_buffer? > > I am adding goto fail for the get_buffer() call in this patch, so I > don't understand what your concern is. > Yes, patch ok. > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavcodec/binkaudio.c b/libavcodec/binkaudio.c index f28ecba760..265f93a822 100644 --- a/libavcodec/binkaudio.c +++ b/libavcodec/binkaudio.c @@ -325,7 +325,7 @@ again: if (s->ch_offset == 0) { frame->nb_samples = s->frame_len; if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) - return ret; + goto fail; if (!new_pkt) frame->pts = AV_NOPTS_VALUE; } @@ -334,8 +334,8 @@ again: avctx->codec->id == AV_CODEC_ID_BINKAUDIO_DCT, FFMIN(MAX_CHANNELS, s->channels - s->ch_offset), s->ch_offset)) { av_log(avctx, AV_LOG_ERROR, "Incomplete packet\n"); - s->ch_offset = 0; - return AVERROR_INVALIDDATA; + ret = AVERROR_INVALIDDATA; + goto fail; } s->ch_offset += MAX_CHANNELS; get_bits_align32(gb);