> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > Andreas Rheinhardt > Sent: 2021年2月21日 12:39 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too > many styles > > Guo, Yejun: > > > > > >> -----Original Message----- > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > >> Andreas Rheinhardt > >> Sent: 2021年2月21日 12:12 > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for > >> too many styles > >> > >> Guo, Yejun: > >>> > >>> > >>>> -----Original Message----- > >>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > >>>> Andreas Rheinhardt > >>>> Sent: 2021年2月21日 9:41 > >>>> To: ffmpeg-devel@ffmpeg.org > >>>> Cc: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > >>>> Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for > >>>> too many styles > >>>> > >>>> The counter for the number of styles is written on two bytes, ergo > >>>> anything > UINT16_MAX is invalid. This also fixes a compiler > >>>> warning because of a tautologically true check on 64bit systems. > >>>> > >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > >>>> --- > >>>> A better solution would be to error out as soon as the byte length > >>>> of a subtitle exceeds UINT16_MAX; yet for this one would have to > >>>> modify all of ass_split to allow the callbacks to return errors. > >>>> > >>>> libavcodec/movtextenc.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c > >>>> index 1bef21e0b9..cf30adbd0a 100644 > >>>> --- a/libavcodec/movtextenc.c > >>>> +++ b/libavcodec/movtextenc.c > >>>> @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext > *s) > >>>> StyleBox *tmp; > >>>> > >>>> // last style != defaults, end the style entry and start a new > one > >>>> - if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) || > >>>> + if (s->count + 1 > FFMIN(SIZE_MAX / > >>>> + sizeof(*s->style_attributes), UINT16_MAX) || > >>> > >>> hi, logically, I think the result of FFMIN(SIZE_MAX / > >> sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we > >> may just use 's->count + 1 > UINT16_MAX'. > >>> > >> > >> And why? > > > > For the original code below, the compiler reports that it is always false. > > s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) > > > > Since s->count is unsigned int, imho, it means that SIZE_MAX / > > sizeof(*s->style_attributes) is always larger than uint_max, and so of > > course > larger than uint16_max. > > > > another is that: > > sizeof(*s->style_attributes) is 12, > > SIZE_MAX: 18446744073709551615 > > UINT16_MAX: 65535 > > > > checked with code: > > #include <stdio.h> > > #include <stdint.h> > > int main() > > { > > printf("SIZE_MAX: %lu\nUINT16_MAX: %d\n", SIZE_MAX, > UINT16_MAX); } > > > FYI: The C standard does not mandate the sizes for most types (the uintx_t > types are an exception). It also gives the compiler absolute freedom on how > much padding to add to a structure. A compiler could very well add megabytes > of padding to StyleBox. The numbers you provide only pertain to one (or > actually none, see below) implementation, not to all of them. It is like > someone claiming to prove a general theorem by only checking it for one > example. > > Several of your statements are btw not only false for a hypothetical system > compliant with the C standard. They are false on common systems: > "SIZE_MAX / sizeof(*s->style_attributes) is always larger than uint_max" > is wrong on ordinary 32bit systems (where SIZE_MAX and UINT_MAX typically > coincide). And "sizeof(*s->style_attributes) is 12". Due to padding it is > normally > 16; it could be reduced to 12 (for ordinary > systems) by rearranging its elements. >
thanks for the lucid explanation, yes, you're right. (and first to know that megabytes of padding is possible) btw, just as a learning, is there possible a real system where uint_max < uint32_max? If no, we might say that: size_max >= uint_max >= uint32_max > uint16_max. _______________________________________________ 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".