Re: [FFmpeg-devel] [PATCH] avformat/asfdec: fix crash caused by free wlid pointers

[XiaoYang <yshaw1999@163.com>]

At 2022-01-12 19:07:44, "Andreas Rheinhardt" <andreas.rheinhardt@outlook.com> wrote:
>XiaoYang:
>> 
>> 
>> 
>> At 2022-01-11 17:29:35, "Andreas Rheinhardt" <andreas.rheinhardt@outlook.com> wrote:
>>> Yang Xiao:
>>>> From: Yang Xiao <yshaw99@outlook.com>
>>>>
>>>> This commit fixed a crash when seeking wma frames, asf decoder will try to demux in function asf_read_pts().
>>>> Pointer member side_data of AVPacket that allocated by stack may be wild pointer.
>>>> Prevent releasing wild pointers in AVPacket when some functions try to call av_packet_unref, example av_read_frame().
>>>> ---
>>>>  libavformat/asfdec_f.c | 2 +-
>>>>  libavformat/mpc.c      | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
>>>> index a8f36ed286..bae0ecfc7c 100644
>>>> --- a/libavformat/asfdec_f.c
>>>> +++ b/libavformat/asfdec_f.c
>>>> @@ -1433,7 +1433,7 @@ static int64_t asf_read_pts(AVFormatContext *s, int stream_index,
>>>>  {
>>>>      FFFormatContext *const si = ffformatcontext(s);
>>>>      ASFContext *asf     = s->priv_data;
>>>> -    AVPacket pkt1, *pkt = &pkt1;
>>>> +    AVPacket pkt1 = {0}, *pkt = &pkt1;
>>>>      ASFStream *asf_st;
>>>>      int64_t pts;
>>>>      int64_t pos = *ppos;
>>>> diff --git a/libavformat/mpc.c b/libavformat/mpc.c
>>>> index b5b2bab33c..ad0d693152 100644
>>>> --- a/libavformat/mpc.c
>>>> +++ b/libavformat/mpc.c
>>>> @@ -189,7 +189,7 @@ static int mpc_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp
>>>>      AVStream *st = s->streams[stream_index];
>>>>      FFStream *const sti = ffstream(st);
>>>>      MPCContext *c = s->priv_data;
>>>> -    AVPacket pkt1, *pkt = &pkt1;
>>>> +    AVPacket pkt1 = {0}, *pkt = &pkt1;
>>>>      int ret;
>>>>      int index = av_index_search_timestamp(st, FFMAX(timestamp - DELAY_FRAMES, 0), flags);
>>>>      uint32_t lastframe;
>>>>
>>>
>>> Do you have FF_API_INIT_PACKET set to 0 (it should still be set to 1)?
>>> Because av_read_frame() is supposed to (and documented to) treat the
>>> packet it is given as uninitialized.
>> 
>>>
>>> - Andreas
>> 
>> 
>> 
>> Thanks for your comment!
>> But I have a question,  av_init_packet() has been markedattribute_deprecated, If av_init_packet() is deprecated completely in the future, how to ensure ff_read_packet() safely (support to input a uninitialized packet) ? AVPacket->side_data will not be safely initialized with only av_packet_unref().
>> 
>
>Uninitialized packets won't be a thing in the future because all packets
>will in the future originate from av_packet_alloc() (save for some in
>libavcodec itself).
>(This does not mean that I like the av_packet_unref(); it'd rather
>document that the packet has to be blank (as if coming from
>av_packet_alloc(), av_packet_unref() or as if it had been used as src
>packet in av_packet_move_ref()).)
>
>> 
>> And as a user of the FFmpeg API, I want to disable some of the functions marked attribute_deprecated early, because upgrading FFmpeg has a large impact. 
>> Do I have other ways to achieve this goal?
>> 
>
>Actually, you are not really supposed to set the FF_API_* defines
>yourself. Although adding a configure option that already disables as
>many of these as possible would be nice for users to test their
>compatibility with future versions as well as for users that don't need
>the old API already.
>

>- Andreas


Thank you for your advice :)
We do this because we can better collect and report problems 
during use new version.

Is there any other effective method unless degrade, such as making 
avpackets allocated by the heap?
_______________________________________________
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".