Anton Khirnov [Fri, 2 Apr 2021 14:33:44 +0000 (16:33 +0200)]
lavc/pngdec: use a separate bytestream reader for each chunk
This makes sure that reading a truncated chunk will never overflow into
the following chunk. It also allows to remove many repeated lines
skipping over the trailing crc checksum.
Anton Khirnov [Sat, 20 Mar 2021 18:57:25 +0000 (19:57 +0100)]
lavc/pngdec: restructure exporting frame meta/side data
This data cannot be stored in PNGDecContext.picture, because the
corresponding chunks may be read after the call to
ff_thread_finish_setup(), at which point modifying shared context data
is a race.
Store intermediate state in the context and then write it directly to
the output frame.
Fixes exporting frame metadata after 5663301560
Fixes #8972
Found-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Anton Khirnov [Fri, 2 Apr 2021 08:45:27 +0000 (10:45 +0200)]
lavc/pngdec: remove unnecessary context variables
Do not store the image buffer pointer/linesize in the context, just
access them directly from the frame.
Stop assuming that linesize is the same for the current and last frame.
ff_vc1_decode_init_alloc_tables() had one error path that forgot to free
already allocated buffers; these would then be overwritten on the next
allocation attempt (or they would just not be freed in case this
happened during init, as the decoders for which it is used do not have
the FF_CODEC_CAP_INIT_CLEANUP set).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/h261dec: Initialize IDCT context during init
Before 998c9f15d1ca8c7489775ebcca51623b915988f1, initializing an
MpegEncContext's IDCT parts occured in ff_mpv_common_init() and this
has been called in h261_decode_frame(), not h261_decode_init().
Yet said commit factored this out of ff_mpv_common_init() and therefore
there is no reason any more not to set this during init as this commit
does.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/rv34, mpegvideo: Fix segfault upon frame size change error
The RealVideo 3.0 and 4.0 decoders call ff_mpv_common_init() only during
their init function and not during decode_frame(); when the size of the
frame changes, they call ff_mpv_common_frame_size_change(). Yet upon
error, said function calls ff_mpv_common_end() which frees the whole
MpegEncContext and not only those parts that
ff_mpv_common_frame_size_change() reinits. As a result, the context will
never be usable again; worse, because decode_frame() contains no check
for whether the context is initialized or not, it is presumed that it is
initialized, leading to segfaults. Basically the same happens if
rv34_decoder_realloc() fails.
This commit fixes this by only resetting the parts that
ff_mpv_common_frame_size_change() changes upon error and by actually
checking whether the context is in need of reinitialization in
ff_rv34_decode_frame().
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/rv10: Don't presume context to be initialized
In case of resolution changes rv20_decode_picture_header() closes and
reopens its MpegEncContext; it checks the latter for errors, yet when
an error happens, it might happen that no new attempt at
reinitialization is performed when decoding the next frame; this leads
to crashes lateron.
This commit fixes this by making sure that initialization will always
be attempted if the context is currently not initialized.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/mpegvideo: Fix memleak upon allocation error
When slice-threading is used, ff_mpv_common_init() duplicates
the first MpegEncContext and allocates some buffers for each
MpegEncContext (the first as well as the copies). But the count of
allocated MpegEncContexts is not updated until after everything has
been allocated and if an error happens after the first one has been
allocated, only the first one is freed; the others leak.
This commit fixes this: The count is now set before the copies are
allocated. Furthermore, the copies are now created and initialized
before the first MpegEncContext, so that the buffers exclusively owned
by each MpegEncContext are still NULL in the src MpegEncContext so
that no double-free happens upon allocation failure.
Given that this effectively touches every line of the init code,
it has also been factored out in a function of its own in order to
remove code duplication with the same code in
ff_mpv_common_frame_size_change() (which was never called when using
more than one slice (and if it were, there would be potential
double-frees)).
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Revert "avcodec: add FF_CODEC_CAP_INIT_CLEANUP for all codecs which use ff_mpv_common_init()"
This mostly reverts commit 4b2863ff01b1fe93d9a518523c9098d17a9d8c6f.
Said commit removed the freeing code from ff_mpv_common_init(),
ff_mpv_common_frame_size_change() and ff_mpeg_framesize_alloc() and
instead added the FF_CODEC_CAP_INIT_CLEANUP to several codecs that use
ff_mpv_common_init(). This introduced several bugs:
a) Several decoders using ff_mpv_common_init() in their init function were
forgotten: This affected FLV, Intel H.263, RealVideo 3.0 and V4.0 as well as
VC-1/WMV3.
b) ff_mpv_common_init() is not only called from the init function of
codecs, it is also called from AVCodec.decode functions. If an error
happens after an allocation has succeeded, it can lead to memleaks;
furthermore, it is now possible for the MpegEncContext to be marked as
initialized even when ff_mpv_common_init() returns an error and this can
lead to segfaults because decoders that call ff_mpv_common_init() when
decoding a frame can mistakenly think that the MpegEncContext has been
properly initialized. This can e.g. happen with H.261 or MPEG-4.
c) Removing code for freeing from ff_mpeg_framesize_alloc() (which can't
be called from any init function) can lead to segfaults because the
check for whether it needs to allocate consists of checking whether the
first of the buffers allocated there has been allocated. This part has
already been fixed in 76cea1d2ce3f23e8131c8664086a1daf873ed694.
d) ff_mpv_common_frame_size_change() can also not be reached from any
AVCodec.init function; yet the changes can e.g. lead to segfaults with
decoders using ff_h263_decode_frame() upon allocation failure, because
the MpegEncContext will upon return be flagged as both initialized and
not in need of reinitialization (granted, the fact that
ff_h263_decode_frame() clears context_reinit before the context has been
reinited is a bug in itself). With the earlier version, the context
would be cleaned upon failure and it would be attempted to initialize
the context again in the next call to ff_h263_decode_frame().
While a) could be fixed by adding the missing FF_CODEC_CAP_INIT_CLEANUP,
keeping the current approach would entail adding cleanup code to several
other places because of b). Therefore ff_mpv_common_init() is again made
to clean up after itself; the changes to the wmv2 decoder and the SVQ1
encoder have not been reverted: The former fixed a memleak, the latter
allowed to remove cleanup code.
Fixes: double free Fixes: ff_free_picture_tables.mp4 Fixes: ff_mpeg_update_thread_context.mp4 Fixes: decode_colskip.mp4 Fixes: memset.mp4 Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
All codec ids on BSF whitelists have a codec descriptor, so one can just
use avcodec_get_name() without worrying about the case of what happens
when no codec descriptor is found.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avutil/frame: Return 0 on success in av_frame_ref()
av_frame_copy() is allowed to return values >= 0 on success, whereas
the documentation of av_frame_ref() states that the return value is 0 on
success. Ergo the latter must not just return the former's return value.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avfilter/vf_find_rect: Use correct format specifier
Fixes the following GCC warning:
warning: format ‘%lld’ expects argument of type ‘long long int’,
but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
Reviewed-by: Gyan Doshi <ffmpeg@gyani.pro> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/mjpegdec: Fix leak in case ICC array allocations fail partially
If only one of the two arrays used for the ICC profile could be
successfully allocated, it might be overwritten and leak when
the next ICC entry is encountered. Fix this by using a common struct,
so that one has only one array to allocate.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/cfhd: Keep track of which subbands have been read
This avoids use of uninitialized data
also several checks are inside the band reading code
so it is important that it is run at least once
Fixes: out of array accesses Fixes: 28209/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5684714694377472 Fixes: 32124/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5425980681355264 Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-4558757155700736 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avcodec/cfhd: Require valid setup before Lowpass coefficients, BandHeader and BandSecondPass
Previously the code skipped all security checks when these where encountered but prior data was incorrect.
Also replace an always true condition by an assert
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array accesses Fixes: 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560 Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512 Fixes: 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avcodec/mjpegdec: Fix leak in case of invalid external Huffman tables
When using external Huffman tables fails during init, the decoder
reverts back to using the default Huffman tables; and when doing so,
the current VLC tables leak because init_default_huffman_tables()
doesn't free them before overwriting them.
avcodec/a64multienc: Don't use static buffers, fix potential races
render_charset() used static buffers that are always completely
initialized before every use, so that it is unnecessary for the
values in these arrays to be kept after leaving the function.
Given that this is not only unnecessary, but harmful due to the
possibility of data races if several instances of a64multi/a64multi5
run simultaneously these buffers have been replaced by ordinary buffers
on the stack (they are small enough for this).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
av_cpu_count() intends to emit a debug message containing the number of
logical cores when called the first time. The check currently works with
a static volatile int; yet this does not help at all in case of
concurrent accesses by multiple threads. So replace this with an
atomic_int.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avcodec/tiff: Don't use separate temporary buffer for fax
Also don't unnecessarily copy the input data around if it needn't be
reversed; and remove a redundant memset -- av_fast_padded_malloc()
already does this for us.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The DSS demuxer currently decrements a counter that should be positive
at the beginning of read_packet; should it become negative, it means
that the data to be read can't be read contiguosly, but has to be read
in two parts. In this case the counter is incremented again after the
first read if said read succeeded; if not, the counter stays negative.
This can lead to problems in further read_packet calls; in tickets #9020
and #9023 it led to segfaults if one tries to seek lateron if the seek
failed and generic seek tried to read from the beginning. But it could
also happen when av_new_packet() failed and the user attempted to read
again afterwards.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
When encoding E-AC-3, whether coupling is on or not determines whether
an additional frame based coupling exponent strategy element frmcplexpstr
(of size five bits) is present in the bitstream. So just add five to the
number of bits when counting them instead of adding 5*s->cpl_on (the
latter field is currently only 0 or 1, so it doesn't make a difference).
Furthermore, move some parts of the bit allocation that doesn't change
per-frame to count_frame_bits_fixed() (which is only run once during
init).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/ac3enc: Avoid needlessly copying encoded packets around
AC-3 and EAC-3 are codecs whose packet sizes are known in advance,
so one can use the min_size parameter of ff_alloc_packet2() to
allocate exactly this amount. This avoids a memcpy later in
av_packet_make_refcounted() in encode_simple_internal().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/ac3enc: Use actual size of buffer in init_put_bits()
Since the very beginning (since de6d9b6404bfd1c589799142da5a95428f146edd)
the AC-3 encoder used AC3_MAX_CODED_FRAME_SIZE (namely 3840) for the
size of the output buffer (without any check at all).
This causes problems when encoding EAC-3 for which the maximum is too small,
smaller than the actual size of the buffer: One can run into asserts used
by the PutBits API. Ticket #8513 is about such a case and this commit
fixes it by using the real size of the buffer.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avformat: Add and use helper function to add attachment streams
All instances of adding attached pictures to a stream or adding
a stream and an attached packet to said stream have several things
in common like setting the index and flags of the packet, setting
the stream disposition etc. This commit therefore factors this out.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
avcodec/qtrleenc: Fix negative linesizes, don't use NULL + offset
Before commit f1e17eb446577180ee9976730aacb46563766518, the qtrle
encoder had undefined pointer arithmetic: Outside of a loop, two
pointers were set to point to the ith element (with index i-1) of
a line of a frame. At the end of each loop iteration, these pointers
were decremented, so that they pointed to the -1th element of the line
after the loop. Furthermore, one of these pointers can be NULL (in which
case all pointer arithmetic is automatically undefined behaviour).
Commit f1e17eb44 added a check in order to ensure that the elements
never point to the -1th element of the array: The pointers are only
decremented if they are bigger than the frame's base pointer
(i.e. AVFrame.data[0]). Yet this check does not work at all in case of
negative linesizes; furthermore in case the pointer that can be NULL is
NULL initializing it still involves undefined pointer arithmetic.
This commit fixes both of these issues: First, non-NULL pointers are
initialized to point to the element after the ith element and
decrementing is moved to the beginning of the loop. Second, if a pointer
is NULL, it is just made to point to the other pointer, as this allows
to avoid checks before decrementing it.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fixes: out of array access Fixes: 32264/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LSCR_fuzzer-6684504010915840 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array write Fixes: 32116/clusterfuzz-testcase-minimized-ffmpeg_dem_SIMBIOSIS_IMX_fuzzer-6702533894602752 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avcodec/h265_metadata_bsf: Check nb_units before accessing the first in h265_metadata_update_fragment()
Fixes: null pointer dereference Fixes: 32113/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_METADATA_fuzzer-4803262287052800
Same as 0c48c332eeb2866d9353125f701e099c48889463
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avformat/rmdec: use larger intermediate type for audio_framesize * sub_packet_h check
Fixes: signed integer overflow: 65535 * 65535 cannot be represented in type 'int' Fixes: 31406/clusterfuzz-testcase-minimized-ffmpeg_dem_IVR_fuzzer-5024692843970560 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
projects / ffmpeg / log