[FFmpeg-devel] libavformat: data loss in message accumulation in fill_buffer()

Submitted by Rob Meyers on May 16, 2017, 4:05 p.m.

Details

Message ID 20170516160549.53653-1-robertmeyers@google.com
State New
Headers show

Commit Message

Rob Meyers May 16, 2017, 4:05 p.m.
We noticed when reading data from a named pipe the first 10 bytes
would get dropped. I traced this to the affected code in
fill_buffer(). The assignment of "dst" was always set to the beginning
of the buffer, and if it hadn't been consumed yet the data would be
overwritten. We could reproduce this by setting up a server that
writes to the named pipe in two small (6 byte) messages with a 1
second gap between. Without the gap, or if the data is sent as one
message, there's no problem. It's in the accumulation of data between
messages to fulfill a read that this bug is triggered.

---
 libavformat/aviobuf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Michael Niedermayer May 16, 2017, 8:25 p.m.
On Tue, May 16, 2017 at 09:05:49AM -0700, Rob Meyers wrote:
> We noticed when reading data from a named pipe the first 10 bytes
> would get dropped. I traced this to the affected code in
> fill_buffer(). The assignment of "dst" was always set to the beginning
> of the buffer, and if it hadn't been consumed yet the data would be
> overwritten. We could reproduce this by setting up a server that
> writes to the named pipe in two small (6 byte) messages with a 1
> second gap between. Without the gap, or if the data is sent as one
> message, there's no problem. It's in the accumulation of data between
> messages to fulfill a read that this bug is triggered.
> 
> ---
>  libavformat/aviobuf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

as just explained in the previous thread, this is not the correct
change to fix the issue
(just saying so this isnt applied by mistake)

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 0a7c39eacd..4e04cb79e0 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -519,9 +519,7 @@  void avio_write_marker(AVIOContext *s, int64_t time, enum AVIODataMarkerType typ
 
 static void fill_buffer(AVIOContext *s)
 {
-    int max_buffer_size = s->max_packet_size ?
-                          s->max_packet_size : IO_BUFFER_SIZE;
-    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size < s->buffer_size ?
+    uint8_t *dst        = !s->max_packet_size && s->buf_end - s->buffer < s->buffer_size ?
                           s->buf_end : s->buffer;
     int len             = s->buffer_size - (dst - s->buffer);