Hi, On Mon, Mar 26, 2012 at 3:20 PM, Janne Grunau <[email protected]> wrote: > From: "Ronald S. Bultje" <[email protected]> > > On 2012-03-26 14:54:56 -0700, Ronald S. Bultje wrote: >> From: "Ronald S. Bultje" <[email protected]> >> >> This will (in ER) write to the pic->mb_type[] array of the previous >> image, which may in a subsequent thread already have been re-used for a >> new image, thus causing two threads to write to the same pic->mb_type[] >> array, causing a race condition which can crash in rv34_decode_cbp(), >> called by rv34_decode_inter_mb_header() (which accesses mb_type[] twice, >> assuming values are maintained, which the race condition breaks). > > Analysis is correct, I think setting s->mb_num_left to 0 after running > ER is cleaner since it also prevents running ER twice on the same frame > when single threaded. > > Janne > ---8<--- > Prevents running error resilience on a previous frame which will write > to the pic->mb_type[] array of the previous image. The array might > already be re-used for a new image in a subsequent thread, thus cause > two threads to write to the same pic->mb_type[] array, causing a race > condition which can crash in rv34_decode_cbp(), called by > rv34_decode_inter_mb_header() (which accesses mb_type[] twice, > assuming values are maintained, which the race condition breaks). > > Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > CC: [email protected] > --- > libavcodec/rv34.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c > index da5d437..b366ead 100644 > --- a/libavcodec/rv34.c > +++ b/libavcodec/rv34.c > @@ -1576,6 +1576,7 @@ static int finish_frame(AVCodecContext *avctx, AVFrame > *pict) > > ff_er_frame_end(s); > ff_MPV_frame_end(s); > + s->mb_num_left = 0; > > if (HAVE_THREADS && (s->avctx->active_thread_type & FF_THREAD_FRAME)) > ff_thread_report_progress(&s->current_picture_ptr->f, INT_MAX, 0); > @@ -1774,6 +1775,7 @@ int ff_rv34_decode_frame(AVCodecContext *avctx, > * only complete frames */ > ff_er_frame_end(s); > ff_MPV_frame_end(s); > + s->mb_num_left = 0; > ff_thread_report_progress(&s->current_picture_ptr->f, INT_MAX, 0); > return AVERROR_INVALIDDATA; > } > -- > 1.7.8.5
Indeed, that's fine also. Feel free to push that if you prefer. Thanks, Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
