diff mbox series

[FFmpeg-devel,3/3] avutil/mem: Use max_alloc_size as-is

Message ID 20200521023647.27847-3-andreas.rheinhardt@gmail.com
State Accepted
Commit 731c77589841c02e014aa7f8285dcfc8b20f2ee5
Headers show
Series [FFmpeg-devel,1/3] avformat/matroskadec: Beautify matroska_parse_laces()
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt May 21, 2020, 2:36 a.m. UTC
The size of a single allocation performed by av_malloc() or av_realloc()
is supposed to be bounded by max_alloc_size, which defaults to INT_MAX
and can be set by the user; yet currently this is not completely
honoured: The actual value used is max_alloc_size - 32. How this came
to be can only be understood historically:

a) 0ecca7a49f8e254c12a3a1de048d738bfbb614c6 disallowed allocations
> INT_MAX. At that time the size parameter of av_malloc() was an
unsigned and the commentary added ("lets disallow possible ambiguous
cases") indicates that this was done as a precaution against calling the
functions with negative int values. Genuinely limiting the size of
allocations to INT_MAX doesn't seem to have been the intention given
that at this time the memalign hack introduced in commit
da9b170c6f06184a5114dc66afb8385cd0ffff83 (which when enabled increased
the size of allocations slightly so that one can return a correctly
aligned pointer that actually does not point to the beginning of the
allocated buffer) was already present.
b) Said memalign hack allocated 17 bytes more than actually desired, yet
allocating 16 bytes more is actually enough and so this was changed in
a9493601638b048c44751956d2360f215918800c; this commit also replaced
INT_MAX by INT_MAX - 16 (and made the limit therefore a limit on the size
of the allocated buffer), but kept the comment, although there is nothing
ambiguous about allocating (INT_MAX - 16)..INT_MAX.
c) 13dfce3d44f99a2d7df71aba8ae003d58db726f7 then increased 16 to 32 for
AVX, 6b4c0be5586acad3bbafd7d2dd02a8328a5ab632 replaced INT_MAX by
MAX_MALLOC_SIZE (which was of course defined to be INT_MAX) and
5a8e994287d8ef181c0a5eac537547d7059b4524 added max_alloc_size and made
it user-selectable.
d) 4fb311c804098d78e5ce5f527f9a9c37536d3a08 then dropped the memalign
hack, yet it kept the -32 (probably because the comment about ambiguous
cases was still present?), although it is no longer needed at all after
this commit. Therefore this commit removes it and uses max_alloc_size
directly.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavutil/mem.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/libavutil/mem.c b/libavutil/mem.c
index a29c224264..cfb6d8ab8f 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -78,8 +78,7 @@  void *av_malloc(size_t size)
 {
     void *ptr = NULL;
 
-    /* let's disallow possibly ambiguous cases */
-    if (size > (max_alloc_size - 32))
+    if (size > max_alloc_size)
         return NULL;
 
 #if HAVE_POSIX_MEMALIGN
@@ -134,8 +133,7 @@  void *av_malloc(size_t size)
 
 void *av_realloc(void *ptr, size_t size)
 {
-    /* let's disallow possibly ambiguous cases */
-    if (size > (max_alloc_size - 32))
+    if (size > max_alloc_size)
         return NULL;
 
 #if HAVE_ALIGNED_MALLOC
@@ -482,12 +480,12 @@  void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size)
     if (min_size <= *size)
         return ptr;
 
-    if (min_size > max_alloc_size - 32) {
+    if (min_size > max_alloc_size) {
         *size = 0;
         return NULL;
     }
 
-    min_size = FFMIN(max_alloc_size - 32, FFMAX(min_size + min_size / 16 + 32, min_size));
+    min_size = FFMIN(max_alloc_size, FFMAX(min_size + min_size / 16 + 32, min_size));
 
     ptr = av_realloc(ptr, min_size);
     /* we could set this to the unmodified min_size but this is safer