diff mbox

[FFmpeg-devel] lavc/avpacket: Fix undefined behaviour, do not pass null pointer to memcpy()

Message ID 201609042115.47744.cehoyos@ag.or.at
State Accepted
Commit f077ad69c682c13ab75a72aec11a61cac53f0c91
Headers show

Commit Message

Carl Eugen Hoyos Sept. 4, 2016, 7:15 p.m. UTC
Hi!

Attached patch fixes ticket #5128.

Please comment, Carl Eugen
From a3dcd65c313e6b456fcb05915508d8d583c1840e Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Sun, 4 Sep 2016 21:11:02 +0200
Subject: [PATCH] lavc/avpacket: Fix undefined behaviour, do not pass a null
 pointer to memcpy().

Fixes ticket #5128.
---
 libavcodec/avpacket.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Nicolas George Sept. 4, 2016, 7:24 p.m. UTC | #1
Le nonidi 19 fructidor, an CCXXIV, Carl Eugen Hoyos a écrit :
> From a3dcd65c313e6b456fcb05915508d8d583c1840e Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> Date: Sun, 4 Sep 2016 21:11:02 +0200
> Subject: [PATCH] lavc/avpacket: Fix undefined behaviour, do not pass a null
>  pointer to memcpy().

I read the spec for memcpy(), passing NULL is not stated as an undefined
behaviour if the size if 0. UBSan seems to be reporting a false positive.

I do not object to the patch, but the commit message could state the bug is
not ours.

Regards,
Carl Eugen Hoyos Sept. 4, 2016, 7:43 p.m. UTC | #2
2016-09-04 21:24 GMT+02:00 Nicolas George <george@nsup.org>:
>> Subject: [PATCH] lavc/avpacket: Fix undefined behaviour,
>> do not pass a null pointer to memcpy().
>
> I read the spec for memcpy(), passing NULL is not stated as an
> undefined behaviour if the size if 0. UBSan seems to be
> reporting a false positive.

This site claims that ISO disagrees:
http://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-perform-memcpy0-0-0

(No opinion whatsoever here, just a ticket.)

Carl Eugen
Nicolas George Sept. 4, 2016, 7:45 p.m. UTC | #3
Le nonidi 19 fructidor, an CCXXIV, Carl Eugen Hoyos a écrit :
> This site claims that ISO disagrees:
> http://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-perform-memcpy0-0-0
> 
> (No opinion whatsoever here, just a ticket.)

I stand corrected.

Regards,
Michael Niedermayer Sept. 4, 2016, 8:43 p.m. UTC | #4
On Sun, Sep 04, 2016 at 09:15:47PM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch fixes ticket #5128.
> 
> Please comment, Carl Eugen

>  avpacket.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 0ddcbcddd2981c7ce86bf15943a1a4065a412b35  0001-lavc-avpacket-Fix-undefined-behaviour-do-not-pass-a-.patch
> From a3dcd65c313e6b456fcb05915508d8d583c1840e Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> Date: Sun, 4 Sep 2016 21:11:02 +0200
> Subject: [PATCH] lavc/avpacket: Fix undefined behaviour, do not pass a null
>  pointer to memcpy().
> 
> Fixes ticket #5128.
> ---
>  libavcodec/avpacket.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM

thx

[...]
Carl Eugen Hoyos Sept. 5, 2016, 8:17 a.m. UTC | #5
2016-09-04 22:43 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Sun, Sep 04, 2016 at 09:15:47PM +0200, Carl Eugen Hoyos wrote:

>> Fixes ticket #5128.
>> ---
>>  libavcodec/avpacket.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> LGTM

Patch applied.

Thank you, Carl Eugen
diff mbox

Patch

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 92186892..fa2844d 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -581,7 +581,8 @@  int av_packet_ref(AVPacket *dst, const AVPacket *src)
         ret = packet_alloc(&dst->buf, src->size);
         if (ret < 0)
             goto fail;
-        memcpy(dst->buf->data, src->data, src->size);
+        if (src->size)
+            memcpy(dst->buf->data, src->data, src->size);
 
         dst->data = dst->buf->data;
     } else {