rtmp: Don't try to do av_malloc(0)

Message ID 1306345057-18757-1-git-send-email-martin@martin.st
State Committed
Commit 271c869cc3285dac2b6f2663a87c70bf3ba2b04f
Headers show

Commit Message

Martin Storsjö May 25, 2011, 5:37 p.m.
Some received packets can have size 0. The return value from
av_malloc(0) may be NULL, which is ok if the size was 0. On
OS X, however, the returned pointer is non-null but leads to
crashes when trying to free it.
---
 libavformat/rtmppkt.c   |    2 ++
 libavformat/rtmpproto.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

Comments

Ronald Bultje May 25, 2011, 5:39 p.m. | #1
Hi,

On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
> @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
>  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
>                           int timestamp, int size)
>  {
> +    if (size) {
>     pkt->data = av_malloc(size);
>     if (!pkt->data)
>         return AVERROR(ENOMEM);
> +    }
>     pkt->data_size  = size;
>     pkt->channel_id = channel_id;
>     pkt->type       = type;

OK.

> @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
>         return AVERROR_EOF;
>
>     for (;;) {
> -        RTMPPacket rpkt;
> +        RTMPPacket rpkt = { 0 };
>         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
>                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
>             if (ret == 0) {

Unrelated? If it fixes something it's OK.

Ronald
Martin Storsjö May 25, 2011, 6:53 p.m. | #2
On Wed, 25 May 2011, Ronald S. Bultje wrote:

> Hi,
> 
> On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
> > @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
> >  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
> >                           int timestamp, int size)
> >  {
> > +    if (size) {
> >     pkt->data = av_malloc(size);
> >     if (!pkt->data)
> >         return AVERROR(ENOMEM);
> > +    }
> >     pkt->data_size  = size;
> >     pkt->channel_id = channel_id;
> >     pkt->type       = type;
> 
> OK.
> 
> > @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
> >         return AVERROR_EOF;
> >
> >     for (;;) {
> > -        RTMPPacket rpkt;
> > +        RTMPPacket rpkt = { 0 };
> >         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
> >                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
> >             if (ret == 0) {
> 
> Unrelated? If it fixes something it's OK.

No, without this, pkt->data will be left uninitialized for these cases 
where size == 0.

I'll push in a moment then - I assume I don't need to post the reindent 
patch before pushing it.

// Mart
Ronald Bultje May 25, 2011, 6:54 p.m. | #3
Hi Martin,

On Wed, May 25, 2011 at 2:53 PM, Martin Storsjö <martin@martin.st> wrote:
> On Wed, 25 May 2011, Ronald S. Bultje wrote:
>> On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
>> > @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
>> >  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
>> >                           int timestamp, int size)
>> >  {
>> > +    if (size) {
>> >     pkt->data = av_malloc(size);
>> >     if (!pkt->data)
>> >         return AVERROR(ENOMEM);
>> > +    }
>> >     pkt->data_size  = size;
>> >     pkt->channel_id = channel_id;
>> >     pkt->type       = type;
>>
>> OK.
>>
>> > @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
>> >         return AVERROR_EOF;
>> >
>> >     for (;;) {
>> > -        RTMPPacket rpkt;
>> > +        RTMPPacket rpkt = { 0 };
>> >         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
>> >                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
>> >             if (ret == 0) {
>>
>> Unrelated? If it fixes something it's OK.
>
> No, without this, pkt->data will be left uninitialized for these cases
> where size == 0.
>
> I'll push in a moment then - I assume I don't need to post the reindent
> patch before pushing it.

Nope, that's fine.

Ronald

Patch

diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
index 63b0628..93790eb 100644
--- a/libavformat/rtmppkt.c
+++ b/libavformat/rtmppkt.c
@@ -233,9 +233,11 @@  int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
 int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
                           int timestamp, int size)
 {
+    if (size) {
     pkt->data = av_malloc(size);
     if (!pkt->data)
         return AVERROR(ENOMEM);
+    }
     pkt->data_size  = size;
     pkt->channel_id = channel_id;
     pkt->type       = type;
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index 70e4b14..f499bd3 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -683,7 +683,7 @@  static int get_packet(URLContext *s, int for_header)
         return AVERROR_EOF;
 
     for (;;) {
-        RTMPPacket rpkt;
+        RTMPPacket rpkt = { 0 };
         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
             if (ret == 0) {