[libav-bugs] [Bug 1142] New: Help me understand why av_interleaved_write_frame fails buffer realloc

bugzilla at libav.org bugzilla at libav.org
Fri Jan 18 09:03:09 CET 2019


https://bugzilla.libav.org/show_bug.cgi?id=1142

            Bug ID: 1142
           Summary: Help me understand why av_interleaved_write_frame
                    fails buffer realloc
           Product: Libav
           Version: git HEAD
          Hardware: X86
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: ---
         Component: libavformat
          Assignee: bugzilla at libav.org
          Reporter: nemesis at icequake.net

I am not sure if this is a bug or misuse of the library.

Well, there is a small bug in src/libavformat/mpegenc.c:

if (av_fifo_realloc2(stream->fifo, av_fifo_size(stream->fifo) + size) < 0)
    return -1;

Because of this, the error was hard to find.  The return value (which can be
AVERROR from the lower frame) should be preserved and returned in the error
case, instead of blocked at this level leaving av_strerror() to mislead the
user.

Anyway:

I am attempting to create a large slideshow with DVDStyler (3000+ images of
similar size).  It fails at the same numbered image every time, regardless of
the actual image file it's processing at the time.

The failure is due to mpeg_mux_write_packet() requesting the AVFifoBuffer to be
reallocated to a size greater than INT_MAX. (Discussion below.)

This is the call stack:
(gdb) bt
#0  0x00007f0a09794888 in fifo_alloc_common (buffer=0x0, size=2147491256) at
src/libavutil/fifo.c:31
#1  0x00007f0a09794939 in av_fifo_alloc (size=2147491256) at
src/libavutil/fifo.c:46
#2  0x00007f0a09794bc1 in av_fifo_realloc2 (f=0x56113894d800,
new_size=2147491256) at src/libavutil/fifo.c:93
#3  0x00007f0a0b2dcfd2 in mpeg_mux_write_packet (ctx=0x561136fdbf40,
pkt=0x7ffc65232fa0) at src/libavformat/mpegenc.c:1187
#4  0x00007f0a0b2f2e56 in write_packet (s=0x561136fdbf40, pkt=0x7ffc65232fa0)
at src/libavformat/mux.c:747
#5  0x00007f0a0b2f4983 in av_interleaved_write_frame (s=0x561136fdbf40,
pkt=0x0) at src/libavformat/mux.c:1231
#6  0x00005611266fb53f in wxFfmpegMediaEncoder::writeVideoFrame()
(this=this at entry=0x7ffc65233470) at mediaenc_ffmpeg.cpp:477
[...]

What's gone wrong here?
1. av_fifo_alloc() calls fifo_alloc_common() with a NULL buffer which fails
2. The NULL buffer had been returned from av_malloc()
3. av_malloc() returned NULL because the size av_fifo_realloc2() provided to
av_fifo_alloc() exceeds INT_MAX, which causes the check in av_malloc() to fail.
4. There doesn't seem to be a reason this allocation should fail, even on a
32-bit host (much less 64-bit) since the av_malloc parameter is a size_t and
the requested allocation is less than SIZE_MAX.  However!  The sanity check in
av_malloc() fails because the maximum size is configured as INT_MAX instead of
SIZE_MAX:

static size_t max_alloc_size= INT_MAX;
[..]
void *av_malloc(size_t size)
{
    void *ptr = NULL;

    /* let's disallow possibly ambiguous cases */
    if (size > (max_alloc_size - 32))
        return NULL;
[..]

5. There is inconsistent usage of "unsigned int" and size_t to compute and
request the desired AVFifoBuffer size.  av_fifo_alloc() and av_fifo_realloc2()
and the other methods in src/libavutil/fifo.c compute the size as an unsigned
int, for example, while the lower-level allocator functions deal in size_t.

There are two results of this:
1. It's impossible to allocate a region greater than 2GB at the av_malloc()
level, regardless of CPU register size.
2. Even if that were possible, the AVFifoBuffer allocation logic is incapable
of requesting a region greater than 4GB even if the CPU had large enough
registers, due to its utilization of 'unsigned int' rather than size_t.

So, that code all smells really bad and probably should be fixed.  If you agree
with my analysis, I can fix it and submit a PR.

But that's not really my question. :-)  My question is actually:

Is there a conceptual reason that, at the level of mpeg_mux_write_packet(),
that a stream should be limited to 2GB, 4GB, or any specific size -- aside from
practical considerations causing the allocation to fail (for example, due to
32-bit size_t, or file writes failing due to underlying filesystem
limitations)?

Separately, am I correct that the client program (DVDStyler) is doing something
wrong by creating a stream that exceeds 2GB which is intended to be written to
a VOB file on a DVD?  Based on my reading, it seems that the client program
should not be doing this and should be taking care to monitor the resulting
stream size from write_packet(), and then generating a fresh VOB stream before
the size would exceed 1GB.

If this is correct, these are my conclusions:
1. DVDStyler's call to av_interleaved_write_frame() is failing in roughly the
correct set of circumstances (writing in excess of 2GB to a MPEG-2 stream
intended for use as a DVD VOB file, which should be limited to 1GB).
2. This failure under a correct set of circumstances is for many wrong reasons.
:-)
3. Additionally, the correct AVERROR (ENOMEM) that would be informational for
the library client to act appropriately, or for the ultimate user to debug the
problem, is thrown away in this particular trace.

Could you please let me know what your thoughts are and where improvements
should be made, if any, based on the above?  For example, what's the correct
way for the client to check the current AVFifoBuffer size corresponding to the
AVFormatContext underlying AVStream it's working with?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.libav.org/pipermail/libav-bugs/attachments/20190118/af970a01/attachment.html>


More information about the libav-bugs mailing list