[Ffmpeg-devel] [PATCH] Fix segfault in bmp decoder

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Sat, Jan 27, 2007 at 02:06:49PM +0100, Michel Bardiaux wrote:
>> The symptom:
>> 
>> ffmpeg -f image2 -i y%06d.bmp -an -y oops.mpg FFmpeg version
>> SVN-r7724, Copyright (c) 2000-2006 Fabrice Bellard, et al. 
>> configuration: libavutil version: 49.2.0 libavcodec version:
>> 51.29.0 libavformat version: 51.8.0 built on Jan 27 2007 12:19:07,
>> gcc: 3.3.5 (Debian 1:3.3.5-13) Input #0, image2, from 'y%06d.bmp': 
>> Duration: 00:01:00.0, start: 0.000000, bitrate: N/A Stream #0.0:
>> Video: bmp, bgr24, 352x288, 25.00 fps(r) Output #0, mpeg, to
>> 'oops.mpg': Stream #0.0: Video: mpeg1video, yuv420p, 352x288,
>> q=2-31, 200 kb/s, 25.00 fps(c) Stream mapping: Stream #0.0 -> #0.0 
>> Press [q] to stop encoding Compiler did not align stack variables.
>> Libavcodec has been miscompiled and may be very slow or crash. This
>> is not a bug in libavcodec, but in the compiler. Do not report
>> crashes to FFmpeg developers. Segmentation fault size=     138kB
>> time=2.0 bitrate= 554.2kbits/s
>> 
>> The syndrome: you have to know, of course, that the message about
>> stack is there for form's sake only, and irrelevant for most
>> crashes... After a number of calls to the decoder, get_buffer
>> returned with a pathological value for p->linesize[0].
>> 
>> The fix: attached.
> 
> looks ok

Its now Monday morning, still not appled, could someone apply please?

> 
> 
>> Note: it is quite likely this patch actually hides a bug in 
>> avcodec_default_get_buffer that causes it to fail without returning
>>  failure status. I am looking into that.
> 
> yes i agree that avcodec_default_get_buffer is likly buggy

The problem there seems to be simply that assert() is ignored:

     assert(INTERNAL_BUFFER_SIZE > s->internal_buffer_count);

Is it OK to change that to av_log plus return(-1)?

> to but either way the buffers must be released ... there also needs
> to be a release_buffer() in "decode_end" which is also missing in
> bmp.c

Isnt that true of *every* codec? But I see png.c pnm.c having no
decode_end. Should I add it there too? And would 8bps.c be a good example?

Anyway, I would rather schedule this after all the things I already have 
going: the change of the bmp decoder to bytestream, and the bmp encoder, 
and the FACT chunk, and the MSGSM codec.

> 
> PS: ive seen alot of mime types on patches but yours had 
> Content-Type: image/x-coreldrawpattern
> 
Weird. When I do a view/source on the copy in my SENT folder, it says
that indeed. One of my config files must be corrupted. Or is it? Maybe 
.pat *is* the extension for coreldrawpattern!


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/