Andreas Rheinhardt:
> The MPEG-PS muxer uses a custom queue of custom packets. To keep track
> of it, it has a pointer (named predecode_packet) to the head of the
> queue and a pointer to where the next packet is to be added (it points
> to the next-pointer of the last element of the queue); furthermore,
> there is also a pointer that points into the queue (called premux_packet).
>
> The exact behaviour was as follows: If premux_packet was NULL when a
> packet is received, it is taken to mean that the old queue is empty and
> a new queue is started. premux_packet will point to the head of said
> queue and the next_packet-pointer points to its next pointer. If
> predecode_packet is NULL, it will also made to point to the newly
> allocated element.
>
> But if premux_packet is NULL and predecode_packet is not, then there
> will be two queues with head elements premux_packet and
> predecode_packet. Yet only elements reachable from predecode_packet are
> ever freed, so the premux_packet queue leaks.
> Worse yet, when the predecode_packet queue will be eventually exhausted,
> predecode_packet will be made to point into the other queue and when
> predecode_packet will be freed, the next pointer of the preceding
> element of the queue will still point to the element just freed. This
> element might very well be still reachable from premux_packet which
> leads to use-after-frees lateron. This happened in the tickets mentioned
> below.
>
> Fix this by never creating two queues in the first place by checking for
> predecode_packet to know whether the queue is empty. If premux_packet is
> NULL, then it is set to the newly allocated element of the queue.
>
> Fixes tickets #6887, #8188 and #8266.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> Disclaimer: I don't know MPEG program streams very well; it might very
> well be that the mere fact that premux_packet can be NULL while
> predecode_packet isn't is indicative of a deeper bug. All I know is that
> this patch only changes behaviour in case the old behaviour was broken
> (i.e. led to leaks or use-after-frees).
>
> libavformat/mpegenc.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index 9bd0a555d4..810dd717ca 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -48,9 +48,9 @@ typedef struct StreamInfo {
> uint8_t id;
> int max_buffer_size; /* in bytes */
> int buffer_index;
> - PacketDesc *predecode_packet;
> + PacketDesc *predecode_packet; /* start of packet queue */
> + PacketDesc *last_packet; /* end of packet queue */
> PacketDesc *premux_packet;
> - PacketDesc **next_packet;
> int packet_number;
> uint8_t lpcm_header[3];
> int lpcm_align;
> @@ -986,6 +986,8 @@ static int remove_decoded_packets(AVFormatContext *ctx,
> int64_t scr)
> }
> stream->buffer_index -= pkt_desc->size;
> stream->predecode_packet = pkt_desc->next;
> + if (!stream->predecode_packet)
> + stream->last_packet = NULL;
> av_freep(&pkt_desc);
> }
> }
> @@ -1177,12 +1179,16 @@ static int mpeg_mux_write_packet(AVFormatContext
> *ctx, AVPacket *pkt)
> av_log(ctx, AV_LOG_TRACE, "dts:%f pts:%f flags:%d stream:%d nopts:%d\n",
> dts / 90000.0, pts / 90000.0, pkt->flags,
> pkt->stream_index, pts != AV_NOPTS_VALUE);
> - if (!stream->premux_packet)
> - stream->next_packet = &stream->premux_packet;
> - *stream->next_packet =
> pkt_desc = av_mallocz(sizeof(PacketDesc));
> if (!pkt_desc)
> return AVERROR(ENOMEM);
> + if (!stream->predecode_packet) {
> + stream->predecode_packet = pkt_desc;
> + } else
> + stream->last_packet->next = pkt_desc;
> + stream->last_packet = pkt_desc;
> + if (!stream->premux_packet)
> + stream->premux_packet = pkt_desc;
> pkt_desc->pts = pts;
> pkt_desc->dts = dts;
>
> @@ -1200,9 +1206,6 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx,
> AVPacket *pkt)
>
> pkt_desc->unwritten_size =
> pkt_desc->size = size;
> - if (!stream->predecode_packet)
> - stream->predecode_packet = pkt_desc;
> - stream->next_packet = &pkt_desc->next;
>
> if (av_fifo_realloc2(stream->fifo, av_fifo_size(stream->fifo) + size) <
> 0)
> return -1;
>
Will apply this patchset tomorrow unless there are objections.
- 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".
