On Tue, Dec 24, 2019 at 01:46:00AM +0000, Andreas Rheinhardt wrote: > Limin Wang: > > On Mon, Dec 23, 2019 at 10:20:37PM -0300, James Almer wrote: > >> On 12/23/2019 8:32 PM, Michael Niedermayer wrote: > >>> On Mon, Dec 23, 2019 at 10:48:13PM +0800, lance.lmw...@gmail.com wrote: > >>>> From: Limin Wang <lance.lmw...@gmail.com> > >>>> > >>>> Signed-off-by: Limin Wang <lance.lmw...@gmail.com> > >>>> --- > >>>> libavutil/frame.c | 7 ++----- > >>>> 1 file changed, 2 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/libavutil/frame.c b/libavutil/frame.c > >>>> index 1d0faec687..0a1ba877cc 100644 > >>>> --- a/libavutil/frame.c > >>>> +++ b/libavutil/frame.c > >>>> @@ -696,11 +696,8 @@ AVFrameSideData > >>>> *av_frame_new_side_data_from_buf(AVFrame *frame, > >>>> if (!buf) > >>>> return NULL; > >>>> > >>>> - if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1) > >>>> - return NULL; > >>>> - > >>>> - tmp = av_realloc(frame->side_data, > >>>> - (frame->nb_side_data + 1) * > >>>> sizeof(*frame->side_data)); > >>>> + tmp = av_realloc_array(frame->side_data, > >>>> + (frame->nb_side_data + 1), > >>>> sizeof(*frame->side_data)); > >>> > >>> does something prevent "frame->nb_side_data + 1" from overflowing ? > >> > >> av_realloc_array() is called with x + 1 as nmemb argument in several > >> places. It checks for "nmemb >= INT_MAX / size", so it will never > >> overflow with a buffer that increases by one element at a time (It would > >> if the check was > alone). > > > > I think about it, in case nb_side_data is INT_MAX, then frame->nb_side_data > > + 1 will overflow already > > before enter av_realloc_array, so I add check for this overflow only. > > > When frame->nb_side_data is INT_MAX - 1, you request to realloc the > array to INT_MAX members. And because of the check James mentioned > this allocation will already fail, hence frame->nb_side_data can never > become INT_MAX (unless it is also set somewhere else in the code). So > no overflow check is necessary in the caller as long as the size of > the array is only increased in steps of 1.
Yes, please ignore my second patch. > > But this relies on undocumented behaviour of av_realloc_array. Maybe > it should be documented? > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
