diff mbox series

[FFmpeg-devel,2/7] avformat/dss: Don't prematurely modify context variable

Message ID HE1PR0301MB21543AA533AC0698DB5D64178F7B9@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Commit afa511ad34452b1806a6cfa2dd785168140843e6
Headers show
Series [FFmpeg-devel,1/7] avformat/utils: Check allocations for failure
Related show

Checks

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

Commit Message

Andreas Rheinhardt April 1, 2021, 9:26 p.m. UTC
The DSS demuxer currently decrements a counter that should be positive
at the beginning of read_packet; should it become negative, it means
that the data to be read can't be read contiguosly, but has to be read
in two parts. In this case the counter is incremented again after the
first read if said read succeeded; if not, the counter stays negative.

This can lead to problems in further read_packet calls; in tickets #9020
and #9023 it led to segfaults if one tries to seek lateron if the seek
failed and generic seek tried to read from the beginning. But it could
also happen when av_new_packet() failed and the user attempted to read
again afterwards.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/dss.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/dss.c b/libavformat/dss.c
index 0585049130..468de3fe84 100644
--- a/libavformat/dss.c
+++ b/libavformat/dss.c
@@ -219,7 +219,6 @@  static int dss_sp_read_packet(AVFormatContext *s, AVPacket *pkt)
     } else
         read_size = DSS_FRAME_SIZE;
 
-    ctx->counter -= read_size;
     ctx->packet_size = DSS_FRAME_SIZE - 1;
 
     ret = av_new_packet(pkt, DSS_FRAME_SIZE);
@@ -231,17 +230,16 @@  static int dss_sp_read_packet(AVFormatContext *s, AVPacket *pkt)
     pkt->stream_index = 0;
     s->bit_rate = 8LL * ctx->packet_size * st->codecpar->sample_rate * 512 / (506 * pkt->duration);
 
-    if (ctx->counter < 0) {
-        int size2 = ctx->counter + read_size;
-
-        ret = avio_read(s->pb, ctx->dss_sp_buf + offset + buff_offset,
-                        size2 - offset);
-        if (ret < size2 - offset)
+    if (ctx->counter < read_size) {
+        ret = avio_read(s->pb, ctx->dss_sp_buf + buff_offset,
+                        ctx->counter);
+        if (ret < ctx->counter)
             goto error_eof;
 
+        offset = ctx->counter;
         dss_skip_audio_header(s, pkt);
-        offset = size2;
     }
+    ctx->counter -= read_size;
 
     ret = avio_read(s->pb, ctx->dss_sp_buf + offset + buff_offset,
                     read_size - offset);
@@ -278,7 +276,7 @@  static int dss_723_1_read_packet(AVFormatContext *s, AVPacket *pkt)
     size = frame_size[byte & 3];
 
     ctx->packet_size = size;
-    ctx->counter -= size;
+    ctx->counter--;
 
     ret = av_new_packet(pkt, size);
     if (ret < 0)
@@ -288,27 +286,26 @@  static int dss_723_1_read_packet(AVFormatContext *s, AVPacket *pkt)
     pkt->data[0]  = byte;
     offset        = 1;
     pkt->duration = 240;
-    s->bit_rate = 8LL * size * st->codecpar->sample_rate * 512 / (506 * pkt->duration);
+    s->bit_rate = 8LL * size-- * st->codecpar->sample_rate * 512 / (506 * pkt->duration);
 
     pkt->stream_index = 0;
 
-    if (ctx->counter < 0) {
-        int size2 = ctx->counter + size;
-
+    if (ctx->counter < size) {
         ret = avio_read(s->pb, pkt->data + offset,
-                        size2 - offset);
-        if (ret < size2 - offset) {
+                        ctx->counter);
+        if (ret < ctx->counter)
             return ret < 0 ? ret : AVERROR_EOF;
-        }
 
+        offset += ctx->counter;
+        size   -= ctx->counter;
+        ctx->counter = 0;
         dss_skip_audio_header(s, pkt);
-        offset = size2;
     }
+    ctx->counter -= size;
 
-    ret = avio_read(s->pb, pkt->data + offset, size - offset);
-    if (ret < size - offset) {
+    ret = avio_read(s->pb, pkt->data + offset, size);
+    if (ret < size)
         return ret < 0 ? ret : AVERROR_EOF;
-    }
 
     return pkt->size;
 }