Message ID | 20210317235958.1308987-2-andreas.rheinhardt@gmail.com |
---|---|
State | Accepted |
Commit | a46d7819051e0e8c61017e75a0389389ae810ca4 |
Headers | show |
Series | [FFmpeg-devel,1/5] avcodec/avpacket: Improve overflow checks when packing dictionary | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On 3/17/2021 8:59 PM, Andreas Rheinhardt wrote: > These are auxiliary side-data functions, so they should have been > switched to size_t in d79e0fe65c51491f9bf8a470bbe36fb09f3e1280, > but this has been forgotten. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/avpacket.c | 13 +++++++++++++ > libavcodec/packet.h | 10 +++++++++- > libavdevice/decklink_dec.cpp | 2 +- > libavdevice/lavfi.c | 2 +- > libavformat/concatdec.c | 2 +- > libavformat/img2dec.c | 3 ++- > libavformat/oggdec.h | 2 +- > 7 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c > index 8f0850fb00..b5bac5c5f2 100644 > --- a/libavcodec/avpacket.c > +++ b/libavcodec/avpacket.c > @@ -507,7 +507,11 @@ int av_packet_split_side_data(AVPacket *pkt){ > } > #endif > > +#if FF_API_BUFFER_SIZE_T > uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) > +#else > +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size) > +#endif > { > uint8_t *data = NULL; > *size = 0; > @@ -526,7 +530,11 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) > > if (pass) > memcpy(data + total_length, str, len); > +#if FF_API_BUFFER_SIZE_T > else if (len > INT_MAX - total_length) > +#else > + else if (len > SIZE_MAX - total_length) > +#endif > return NULL; > total_length += len; > } > @@ -542,7 +550,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) > return data; > } > > +#if FF_API_BUFFER_SIZE_T > int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict) > +#else > +int av_packet_unpack_dictionary(const uint8_t *data, size_t size, > + AVDictionary **dict) > +#endif You can use buffer_size_t to reduce the ifdeffery in both cases. > { > const uint8_t *end; > int ret; > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index da4377e09f..ca18ae631f 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -647,7 +647,11 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type); > * @param size pointer to store the size of the returned data > * @return pointer to data if successful, NULL otherwise > */ > +#if FF_API_BUFFER_SIZE_T > uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size); > +#else > +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size); > +#endif > /** > * Unpack a dictionary from side_data. > * > @@ -656,8 +660,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size); > * @param dict the metadata storage dictionary > * @return 0 on success, < 0 on failure > */ > +#if FF_API_BUFFER_SIZE_T > int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict); > - > +#else > +int av_packet_unpack_dictionary(const uint8_t *data, size_t size, > + AVDictionary **dict); > +#endif > > /** > * Convenience function to free all the side data stored. > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp > index 40c3dae968..79d96cd620 100644 > --- a/libavdevice/decklink_dec.cpp > +++ b/libavdevice/decklink_dec.cpp > @@ -922,7 +922,6 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > const char *tc = av_timecode_make_string(&tcr, tcstr, 0); > if (tc) { > AVDictionary* metadata_dict = NULL; > - int metadata_len; > uint8_t* packed_metadata; > > if (av_cmp_q(ctx->video_st->r_frame_rate, av_make_q(60, 1)) < 1) { > @@ -937,6 +936,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > } > > if (av_dict_set(&metadata_dict, "timecode", tc, 0) >= 0) { > + buffer_size_t metadata_len; > packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len); > av_dict_free(&metadata_dict); > if (packed_metadata) { > diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c > index 94ad03268a..fdadff3f7f 100644 > --- a/libavdevice/lavfi.c > +++ b/libavdevice/lavfi.c > @@ -446,7 +446,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) > > frame_metadata = frame->metadata; > if (frame_metadata) { > - int size; > + buffer_size_t size; > uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size); > > if (!metadata) { > diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c > index 6d5b9914f9..32d4a99010 100644 > --- a/libavformat/concatdec.c > +++ b/libavformat/concatdec.c > @@ -626,7 +626,7 @@ static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt) > av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &st->time_base), > av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &st->time_base)); > if (cat->cur_file->metadata) { > - int metadata_len; > + buffer_size_t metadata_len; > char* packed_metadata = av_packet_pack_dictionary(cat->cur_file->metadata, &metadata_len); > if (!packed_metadata) > return AVERROR(ENOMEM); > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > index 6c78dada8c..be7149bb2f 100644 > --- a/libavformat/img2dec.c > +++ b/libavformat/img2dec.c > @@ -381,9 +381,10 @@ int ff_img_read_header(AVFormatContext *s1) > * as a dictionary, so it can be used by filters like 'drawtext'. > */ > static int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt) { > - int metadata_len, ret; > AVDictionary *d = NULL; > char *packed_metadata = NULL; > + buffer_size_t metadata_len; > + int ret; > > av_dict_set(&d, "lavf.image2dec.source_path", filename, 0); > av_dict_set(&d, "lavf.image2dec.source_basename", av_basename(filename), 0); > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > index 629a1d6262..bf982bfe1c 100644 > --- a/libavformat/oggdec.h > +++ b/libavformat/oggdec.h > @@ -87,7 +87,7 @@ struct ogg_stream { > int start_trimming; ///< set the number of packets to drop from the start > int end_trimming; ///< set the number of packets to drop from the end > uint8_t *new_metadata; > - unsigned int new_metadata_size; > + buffer_size_t new_metadata_size; > void *private; > }; LGTM either way.
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index 8f0850fb00..b5bac5c5f2 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -507,7 +507,11 @@ int av_packet_split_side_data(AVPacket *pkt){ } #endif +#if FF_API_BUFFER_SIZE_T uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) +#else +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size) +#endif { uint8_t *data = NULL; *size = 0; @@ -526,7 +530,11 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) if (pass) memcpy(data + total_length, str, len); +#if FF_API_BUFFER_SIZE_T else if (len > INT_MAX - total_length) +#else + else if (len > SIZE_MAX - total_length) +#endif return NULL; total_length += len; } @@ -542,7 +550,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size) return data; } +#if FF_API_BUFFER_SIZE_T int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict) +#else +int av_packet_unpack_dictionary(const uint8_t *data, size_t size, + AVDictionary **dict) +#endif { const uint8_t *end; int ret; diff --git a/libavcodec/packet.h b/libavcodec/packet.h index da4377e09f..ca18ae631f 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -647,7 +647,11 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type); * @param size pointer to store the size of the returned data * @return pointer to data if successful, NULL otherwise */ +#if FF_API_BUFFER_SIZE_T uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size); +#else +uint8_t *av_packet_pack_dictionary(AVDictionary *dict, size_t *size); +#endif /** * Unpack a dictionary from side_data. * @@ -656,8 +660,12 @@ uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size); * @param dict the metadata storage dictionary * @return 0 on success, < 0 on failure */ +#if FF_API_BUFFER_SIZE_T int av_packet_unpack_dictionary(const uint8_t *data, int size, AVDictionary **dict); - +#else +int av_packet_unpack_dictionary(const uint8_t *data, size_t size, + AVDictionary **dict); +#endif /** * Convenience function to free all the side data stored. diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp index 40c3dae968..79d96cd620 100644 --- a/libavdevice/decklink_dec.cpp +++ b/libavdevice/decklink_dec.cpp @@ -922,7 +922,6 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( const char *tc = av_timecode_make_string(&tcr, tcstr, 0); if (tc) { AVDictionary* metadata_dict = NULL; - int metadata_len; uint8_t* packed_metadata; if (av_cmp_q(ctx->video_st->r_frame_rate, av_make_q(60, 1)) < 1) { @@ -937,6 +936,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( } if (av_dict_set(&metadata_dict, "timecode", tc, 0) >= 0) { + buffer_size_t metadata_len; packed_metadata = av_packet_pack_dictionary(metadata_dict, &metadata_len); av_dict_free(&metadata_dict); if (packed_metadata) { diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 94ad03268a..fdadff3f7f 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -446,7 +446,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) frame_metadata = frame->metadata; if (frame_metadata) { - int size; + buffer_size_t size; uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size); if (!metadata) { diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index 6d5b9914f9..32d4a99010 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -626,7 +626,7 @@ static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt) av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &st->time_base), av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &st->time_base)); if (cat->cur_file->metadata) { - int metadata_len; + buffer_size_t metadata_len; char* packed_metadata = av_packet_pack_dictionary(cat->cur_file->metadata, &metadata_len); if (!packed_metadata) return AVERROR(ENOMEM); diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c index 6c78dada8c..be7149bb2f 100644 --- a/libavformat/img2dec.c +++ b/libavformat/img2dec.c @@ -381,9 +381,10 @@ int ff_img_read_header(AVFormatContext *s1) * as a dictionary, so it can be used by filters like 'drawtext'. */ static int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt) { - int metadata_len, ret; AVDictionary *d = NULL; char *packed_metadata = NULL; + buffer_size_t metadata_len; + int ret; av_dict_set(&d, "lavf.image2dec.source_path", filename, 0); av_dict_set(&d, "lavf.image2dec.source_basename", av_basename(filename), 0); diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h index 629a1d6262..bf982bfe1c 100644 --- a/libavformat/oggdec.h +++ b/libavformat/oggdec.h @@ -87,7 +87,7 @@ struct ogg_stream { int start_trimming; ///< set the number of packets to drop from the start int end_trimming; ///< set the number of packets to drop from the end uint8_t *new_metadata; - unsigned int new_metadata_size; + buffer_size_t new_metadata_size; void *private; };
These are auxiliary side-data functions, so they should have been switched to size_t in d79e0fe65c51491f9bf8a470bbe36fb09f3e1280, but this has been forgotten. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/avpacket.c | 13 +++++++++++++ libavcodec/packet.h | 10 +++++++++- libavdevice/decklink_dec.cpp | 2 +- libavdevice/lavfi.c | 2 +- libavformat/concatdec.c | 2 +- libavformat/img2dec.c | 3 ++- libavformat/oggdec.h | 2 +- 7 files changed, 28 insertions(+), 6 deletions(-)