[08/10] avpacket: Check the right, actual buffer pointer after av_buffer_realloc

Message ID 1378932481-98398-8-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Sept. 11, 2013, 8:47 p.m.
Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
CC: libav-stable@libav.org
---
 libavcodec/avpacket.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Diego Biurrun Sept. 11, 2013, 10:33 p.m. | #1
On Wed, Sep 11, 2013 at 11:47:59PM +0300, Martin Storsjö wrote:
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -68,7 +68,7 @@ static int packet_alloc(AVBufferRef **buf, int size)
>  
>      av_buffer_realloc(buf, size + FF_INPUT_BUFFER_PADDING_SIZE);
> -    if (!buf)
> +    if (!*buf)
>          return AVERROR(ENOMEM);

av_buffer_realloc() returns a negative AVERROR on failure.  It would
seem safer to check that instead of the pointer, as is done in other
places.  If I read the code correctly, av_buffer_realloc may fail w/o
setting buf to NULL.

Diego
Martin Storsjö Sept. 12, 2013, 8:40 a.m. | #2
On Thu, 12 Sep 2013, Diego Biurrun wrote:

> On Wed, Sep 11, 2013 at 11:47:59PM +0300, Martin Storsjö wrote:
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -68,7 +68,7 @@ static int packet_alloc(AVBufferRef **buf, int size)
>>
>>      av_buffer_realloc(buf, size + FF_INPUT_BUFFER_PADDING_SIZE);
>> -    if (!buf)
>> +    if (!*buf)
>>          return AVERROR(ENOMEM);
>
> av_buffer_realloc() returns a negative AVERROR on failure.  It would
> seem safer to check that instead of the pointer, as is done in other
> places.  If I read the code correctly, av_buffer_realloc may fail w/o
> setting buf to NULL.

That's probably a better option. I'd still like to have an ack from Anton 
on this matter though.

// Martin

Patch

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 79123b1..274f293 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -68,7 +68,7 @@  static int packet_alloc(AVBufferRef **buf, int size)
         return AVERROR(EINVAL);
 
     av_buffer_realloc(buf, size + FF_INPUT_BUFFER_PADDING_SIZE);
-    if (!buf)
+    if (!*buf)
         return AVERROR(ENOMEM);
 
     memset((*buf)->data + size, 0, FF_INPUT_BUFFER_PADDING_SIZE);