[FFmpeg-devel] [PATCH] Fix off-by-few crasher in ff_h2645_extract_rbsp function
nfxjfg at googlemail.com
Tue Mar 7 13:15:15 EET 2017
On Tue, 7 Mar 2017 11:55:23 +0100
Michał Krasowski <mkrasowski at opera.com> wrote:
> @Michael Niedermayer
> I have read the documentation, namely
> * @ingroup lavc_decoding
> * Required number of additionally allocated bytes at the end of the input
> bitstream for decoding.
> * This is mainly needed because some optimized bitstream readers read
> * 32 or 64 bit at once and could read over the end.<br>
> * Note: If the first 23 bits of the additional bytes are not 0, then
> * MPEG bitstreams could cause overread and segfault.
I think the MPEG comment is misleading - this applies to all codecs.
Also, as someone said, the padding is in bytes, but these comments
above make it confusing by talking about bits.
> #define AV_INPUT_BUFFER_PADDING_SIZE 32
> and now it seems to me that you prefer speed (a.k.a. optimization)
> over having a self-contained functions.
> There are few things that are still not clear to me:
> * Why is the 32 bit padding used when the doc says that
> 64 bit offset may also be needed?
> * Even if I extend my data buffer
> to have those 4 bytes more, is there a risk that some functions
> in ffmpeg will read out-of-bounds?
> * How to find such information without reading all bolts and nuts of ffmpeg
I sort of agree. Normally, the AVPacket functions guarantee the
padding. So you "only" need to be careful when you setup an AVPacket
I wonder if we could handle this transparently by checking the
AVPacket.buf allocation. We don't really support non-refcounted input
with the new decode/encode APIs anyway, so this would probably be
possible. Of course it'd mean that the data gets copied if the user
doesn't supply the padding.
More information about the ffmpeg-devel