| Message ID | CAB0OVGrMWB_xG+kKDUYjr0mab71gzHQbt0OMs+iL7wzAGKvrJA@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 12/30/2017 1:44 PM, Carl Eugen Hoyos wrote: > FFmpeg has an arbitrary allocation limit (2G iirc), av_fast_realloc() > increases the allocation even if the requested is equal the already > allocated size. I believe this can lead to unnecessary OOM (no > testcase) if the requested (and already allocated) size is close to > our limit. > Additionally, this avoids an over-allocation for the mov stts patch I just sent. > Attached patch changes the behaviour introduced 15 years ago. I'm not aware of such a limit within the libraries (there is no allocation tracking)? The change itself looks like it is correct to me, unless there is some special reason it was < instead of <= before. - Derek
2017-12-30 17:24 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 12/30/2017 1:44 PM, Carl Eugen Hoyos wrote: >> FFmpeg has an arbitrary allocation limit (2G iirc), av_fast_realloc() >> increases the allocation even if the requested is equal the already >> allocated size. I believe this can lead to unnecessary OOM (no >> testcase) if the requested (and already allocated) size is close to >> our limit. >> Additionally, this avoids an over-allocation for the mov stts patch I just sent. >> Attached patch changes the behaviour introduced 15 years ago. > I'm not aware of such a limit within the libraries (there is no allocation > tracking)? I just confirmed the ("arbitrary") limit defaults to INT_MAX which at least on some systems is 2G as claimed above. Carl Eugen
On 12/30/2017 5:02 PM, Carl Eugen Hoyos wrote: > I just confirmed the ("arbitrary") limit defaults to INT_MAX which at least > on some systems is 2G as claimed above. Right, but this is system-dependent, and not specific to FFmpeg, which was what I meant. I digress though, this is off-topic from the patch at hand, which looked OK to me (barring any special reason for < having been used, which git blame found nothing for). - Derek
On 12/30/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Hi! > > FFmpeg has an arbitrary allocation limit (2G iirc), av_fast_realloc() > increases the allocation even if the requested is equal the already > allocated size. I believe this can lead to unnecessary OOM (no > testcase) if the requested (and already allocated) size is close to > our limit. > Additionally, this avoids an over-allocation for the mov stts patch I just > sent. > Attached patch changes the behaviour introduced 15 years ago. > > Please comment, Carl Eugen > LGTM
On 12/30/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-12-30 17:24 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: >> On 12/30/2017 1:44 PM, Carl Eugen Hoyos wrote: >>> FFmpeg has an arbitrary allocation limit (2G iirc), av_fast_realloc() >>> increases the allocation even if the requested is equal the already >>> allocated size. I believe this can lead to unnecessary OOM (no >>> testcase) if the requested (and already allocated) size is close to >>> our limit. >>> Additionally, this avoids an over-allocation for the mov stts patch I >>> just sent. >>> Attached patch changes the behaviour introduced 15 years ago. > >> I'm not aware of such a limit within the libraries (there is no allocation >> tracking)? > > I just confirmed the ("arbitrary") limit defaults to INT_MAX which at least > on some systems is 2G as claimed above. > Will you apply this or not?
2018-01-01 18:47 GMT+01:00 Paul B Mahol <onemda@gmail.com>: > On 12/30/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2017-12-30 17:24 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: >>> On 12/30/2017 1:44 PM, Carl Eugen Hoyos wrote: >>>> FFmpeg has an arbitrary allocation limit (2G iirc), av_fast_realloc() >>>> increases the allocation even if the requested is equal the already >>>> allocated size. I believe this can lead to unnecessary OOM (no >>>> testcase) if the requested (and already allocated) size is close to >>>> our limit. >>>> Additionally, this avoids an over-allocation for the mov stts patch I >>>> just sent. >>>> Attached patch changes the behaviour introduced 15 years ago. >> >>> I'm not aware of such a limit within the libraries (there is no allocation >>> tracking)? >> >> I just confirmed the ("arbitrary") limit defaults to INT_MAX which at least >> on some systems is 2G as claimed above. > > Will you apply this or not? The patch makes no difference for the committed version of the stts patch but it may still be a good idea, therefore applied. Thank you, Carl Eugen
From 0ad7a0517c69d04a4443e66eeec802bda21aea55 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Sat, 30 Dec 2017 14:38:33 +0100 Subject: [PATCH] lavu/mem: Do not realloc in av_fast_realloc() if size == min_size. This can avoid OOM for min_size close to FFmpeg's arbitrary alloc limits. --- libavutil/mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/mem.c b/libavutil/mem.c index 79e8b59..0729e1d 100644 --- a/libavutil/mem.c +++ b/libavutil/mem.c @@ -463,7 +463,7 @@ void av_memcpy_backptr(uint8_t *dst, int back, int cnt) void *av_fast_realloc(void *ptr, unsigned int *size, size_t min_size) { - if (min_size < *size) + if (min_size <= *size) return ptr; min_size = FFMAX(min_size + min_size / 16 + 32, min_size); -- 1.7.10.4