[FFmpeg-devel] [PATCH] avcodec/cbs: Allocate more CodedBitstreamUnit at once in cbs_insert_unit()
Michael Niedermayer
michael at niedermayer.cc
Sat Apr 11 02:53:25 EEST 2020
On Fri, Apr 10, 2020 at 05:44:25PM -0300, James Almer wrote:
> On 4/10/2020 5:23 PM, Michael Niedermayer wrote:
> > Fixes: Timeout (85sec -> 0.5sec)
> > Fixes: 20791/clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_FRAME_SPLIT_fuzzer-5659537719951360
> > Fixes: 21214/clusterfuzz-testcase-minimized-ffmpeg_BSF_MPEG2_METADATA_fuzzer-5165560875974656
> > Fixes: 21247/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5715175257931776
> >
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> > libavcodec/cbs.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> > index 0bd5e1ac5d..42cb9711fa 100644
> > --- a/libavcodec/cbs.c
> > +++ b/libavcodec/cbs.c
> > @@ -693,11 +693,11 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
> > memmove(units + position + 1, units + position,
> > (frag->nb_units - position) * sizeof(*units));
> > } else {
> > - units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
> > + units = av_malloc_array(frag->nb_units*2 + 1, sizeof(*units));
> > if (!units)
> > return AVERROR(ENOMEM);
> >
> > - ++frag->nb_units_allocated;
> > + frag->nb_units_allocated = 2*frag->nb_units_allocated + 1;
>
> Use ff_fast_malloc(), please. This is quite ugly and the *2 undocumented
> and not obvious.
not sure i understood your suggestion correctly but
ff_fast_malloc() deallocates its buffer and clears the size on error,
which cbs_insert_unit() does not do.
so using ff_fast_malloc() feels a bit like fitting a square peg in a round
hole. But it works too
is that what you had in mind ? (your comment sounded like something simpler ...)
so maybe iam missing something
From 10f4caf17ebf695112777dd73978a2992d0f78cc Mon Sep 17 00:00:00 2001
From: Michael Niedermayer <michael at niedermayer.cc>
Date: Fri, 10 Apr 2020 22:05:07 +0200
Subject: [PATCH] avcodec/cbs: Allocate more CodedBitstreamUnit at once in
cbs_insert_unit()
Fixes: Timeout (85sec -> 0.5sec)
Fixes: 20791/clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_FRAME_SPLIT_fuzzer-5659537719951360
Fixes: 21214/clusterfuzz-testcase-minimized-ffmpeg_BSF_MPEG2_METADATA_fuzzer-5165560875974656
Fixes: 21247/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5715175257931776
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
---
libavcodec/cbs.c | 21 +++++++++++----------
libavcodec/cbs.h | 4 ++--
libavcodec/utils.c | 4 ++--
libavutil/mem.c | 4 ++--
libavutil/mem_internal.h | 17 +++++++++++------
5 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 0bd5e1ac5d..4a1393cb44 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -23,6 +23,7 @@
#include "libavutil/avassert.h"
#include "libavutil/buffer.h"
#include "libavutil/common.h"
+#include "libavutil/mem_internal.h"
#include "cbs.h"
#include "cbs_internal.h"
@@ -684,27 +685,27 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
int position)
{
- CodedBitstreamUnit *units;
+ CodedBitstreamUnit *units = NULL;
+ size_t new_size;
- if (frag->nb_units < frag->nb_units_allocated) {
- units = frag->units;
+ if (av_size_mult(frag->nb_units + 1, sizeof(*units), &new_size))
+ return AVERROR(ENOMEM);
- if (position < frag->nb_units)
- memmove(units + position + 1, units + position,
- (frag->nb_units - position) * sizeof(*units));
- } else {
- units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
+ if (ff_fast_malloc(&units, &frag->nb_units_allocated, new_size, 0, 0)) {
if (!units)
return AVERROR(ENOMEM);
- ++frag->nb_units_allocated;
-
if (position > 0)
memcpy(units, frag->units, position * sizeof(*units));
if (position < frag->nb_units)
memcpy(units + position + 1, frag->units + position,
(frag->nb_units - position) * sizeof(*units));
+ } else {
+ units = frag->units;
+ if (position < frag->nb_units)
+ memmove(units + position + 1, units + position,
+ (frag->nb_units - position) * sizeof(*units));
}
memset(units + position, 0, sizeof(*units));
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 9ca1fbd609..b496ade74e 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -149,9 +149,9 @@ typedef struct CodedBitstreamFragment {
/**
* Number of allocated units.
*
- * Must always be >= nb_units; designed for internal use by cbs.
+ * Must always be >= nb_units; designed for internal use by cbs and *_fast_malloc().
*/
- int nb_units_allocated;
+ unsigned int nb_units_allocated;
/**
* Pointer to an array of units of length nb_units_allocated.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 26c038dfd9..d7f886a9b6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -75,7 +75,7 @@ void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size)
*size = 0;
return;
}
- if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1))
+ if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1, 1))
memset(*p + min_size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
}
@@ -87,7 +87,7 @@ void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size)
*size = 0;
return;
}
- if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1))
+ if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1, 1))
memset(*p, 0, min_size + AV_INPUT_BUFFER_PADDING_SIZE);
}
diff --git a/libavutil/mem.c b/libavutil/mem.c
index 88fe09b179..56e3628178 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -499,10 +499,10 @@ void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size)
void av_fast_malloc(void *ptr, unsigned int *size, size_t min_size)
{
- ff_fast_malloc(ptr, size, min_size, 0);
+ ff_fast_malloc(ptr, size, min_size, 0, 1);
}
void av_fast_mallocz(void *ptr, unsigned int *size, size_t min_size)
{
- ff_fast_malloc(ptr, size, min_size, 1);
+ ff_fast_malloc(ptr, size, min_size, 1, 1);
}
diff --git a/libavutil/mem_internal.h b/libavutil/mem_internal.h
index 6fdbcb016e..cadbba25ec 100644
--- a/libavutil/mem_internal.h
+++ b/libavutil/mem_internal.h
@@ -24,22 +24,27 @@
#include "avassert.h"
#include "mem.h"
-static inline int ff_fast_malloc(void *ptr, unsigned int *size, size_t min_size, int zero_realloc)
+static inline int ff_fast_malloc(void *ptr, unsigned int *size, size_t min_size, int zero_realloc, int do_free)
{
void *val;
memcpy(&val, ptr, sizeof(val));
if (min_size <= *size) {
- av_assert0(val || !min_size);
+ if (do_free)
+ av_assert0(val || !min_size);
return 0;
}
min_size = FFMAX(min_size + min_size / 16 + 32, min_size);
- av_freep(ptr);
+ if (do_free)
+ av_freep(ptr);
val = zero_realloc ? av_mallocz(min_size) : av_malloc(min_size);
memcpy(ptr, &val, sizeof(val));
- if (!val)
- min_size = 0;
- *size = min_size;
+ if (do_free) {
+ if (!val)
+ min_size = 0;
+ *size = min_size;
+ } else if (val)
+ *size = min_size;
return 1;
}
#endif /* AVUTIL_MEM_INTERNAL_H */
--
2.17.1
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200411/c02ce8b4/attachment.sig>
More information about the ffmpeg-devel
mailing list