[FFmpeg-devel] [PATCH 1/3] ffmdec: reset packet_end in case of failure

[Michael Niedermayer]

On Sat, Jan 02, 2016 at 07:43:03PM +0100, Andreas Cadhalpun wrote:
> On 02.01.2016 19:14, Michael Niedermayer wrote:
> > On Sat, Jan 02, 2016 at 04:51:17PM +0100, Andreas Cadhalpun wrote:
> >> This fixes segmentation faults caused by passing a packet_ptr of NULL to
> >> memcpy.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavformat/ffmdec.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c
> >> index 9fe4155..7b2d0d7 100644
> >> --- a/libavformat/ffmdec.c
> >> +++ b/libavformat/ffmdec.c
> >> @@ -123,8 +123,10 @@ static int ffm_read_data(AVFormatContext *s,
> >>              frame_offset = avio_rb16(pb);
> >>              avio_read(pb, ffm->packet, ffm->packet_size - FFM_HEADER_SIZE);
> >>              ffm->packet_end = ffm->packet + (ffm->packet_size - FFM_HEADER_SIZE - fill_size);
> >> -            if (ffm->packet_end < ffm->packet || frame_offset < 0)
> >> +            if (ffm->packet_end < ffm->packet || frame_offset < 0) {
> >> +                ffm->packet_end = ffm->packet_ptr;
> > 
> > doesnt this imply that packet_end was set to a invalid pointer?
> 
> Yes, if you use a strict definition of a valid pointer.
> (It could still point to a valid memory address, but from a different
> memory allocation than packet_ptr.)
> 
> By the way, the check for frame_offset < 0 is pointless, because
> avio_rb16 returns an unsigned int.
> 
> > (that is undefined behavior)
> 
> Yes, but ubsan didn't catch it. ;)
> 
> Anyway, attached is an updated patch avoiding this problem.
> 
> Best regards,
> Andreas

>  ffmdec.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 1600298d54df3b7c7c74d4437521405b38342c60  0001-ffmdec-reset-packet_end-in-case-of-failure.patch
> From a0faebf31ab37083e140c6d276b16dd024f97ffb Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Sat, 2 Jan 2016 16:27:02 +0100
> Subject: [PATCH 1/3] ffmdec: reset packet_end in case of failure
> 
> This fixes segmentation faults caused by passing a packet_ptr of NULL to
> memcpy.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>

should be ok

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160103/d441495d/attachment.sig>