On Sun, Nov 08, 2020 at 11:35:43AM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Fri, Oct 16, 2020 at 10:57:22AM +0200, Joakim Tjernlund wrote: > >> From https://bugs.chromium.org/p/chromium/issues/detail?id=1095962 > >> ---------------------------- > >> This seems to be caused by the custom handling of "av_max_alloc(0)" in > >> Chromium's ffmpeg fork to mean unlimited (added in [1]). > >> > >> Upstream ffmpeg doesn't treat 0 as a special value; versions before 4.3 > >> seemingly worked > >> because 32 was subtracted from max_alloc_size (set to 0 by Chromium) > >> resulting in an > >> integer underflow, making the effective limit be SIZE_MAX - 31. > >> > >> Now that the above underflow doesn't happen, the tab just crashes. The > >> upstream change > >> for no longer subtracting 32 from max_alloc_size was included in ffmpeg > >> 4.3. [2] > >> > >> [1] > >> https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/73563 > >> [2] https://github.com/FFmpeg/FFmpeg/commit/731c77589841 > >> --------------------------- > >> > >> Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older > >> chromium etc. > >> > >> Signed-off-by: Joakim Tjernlund <joakim.tjernl...@infinera.com> > >> --- > >> > >> v2: Cover the full API range 0-31 > >> > >> v3: Closer compat with < 4.3 ffmpeg > >> > >> v4: Adjust size accoriding to Andreas Rheinhardt comments > >> > >> libavutil/mem.c | 2 ++ > >> 1 file changed, 2 insertions(+) > > > > "Unbreak av_malloc_max(0) API/ABI" > > The commit message of this is incorrect > > > > The API before and after this documented the current git master behavior > > more correct would be > > "Break API of av_malloc_max() for 0-31, to restore ABI behavior so > > applications using this do not need to be changed" > > > > I agree. > > > Also id like to raise awareness that this function had a big warning: > > * > > * @warning Exercise extreme caution when using this function. Don't > > touch > > * this if you do not understand the full consequence of doing > > so. > > */ > > void av_max_alloc(size_t max); > > > > And also id like to raise awareness that the default limit is INT_MAX > > while with 0 as argument it becomes SIZE_MAX. > > No: With the old version 0 was effectively SIZE_MAX - 31, with current > git head 0 means that all allocations of > 0 fail (yet requests of size > 0 still result in an allocation of size 1); with the proposed version > av_max_alloc(0) would set the limit to SIZE_MAX - 32. This off-by-one is > surely unintentional. >
> I would expect that is > > not safe everywhere and could open security issues. > > If anything 0 should be interpreted as the default INT_MAX > > That would be an API break. yes, that was meant as a "less bad" alternative to SIZE_MAX - 31 > > > If its not obvious where SIZE_MAX can be an issue, consider what we > > use to index arrays, int, and that doesnt go to SIZE_MAX but instead > > hits undefined behavior maybe becomes negative and accesses out of array > > Any code that relies on allocations > INT_MAX to fail is buggy and must > be fixed; YES > and so is any code that uses an index parameter of type int > and uses a comparison with a value of type size_t as its loop condition. YES though there are more complex failure cases for example a array[i + j*C] here the 2 loops for i and j can have int limits and int indexes and still if the array is not limited to INT_MAX this could go bad > > - Andreas > > *: Btw: AVFormatContext.nb_streams is unsigned, yet it is common to use > an int to loop over the streams. This is not dangerous now, as the > max_streams option is limited to INT_MAX. But we should probably change > this habit. yes thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
