| Message ID | 20201015112024.6521-1-joakim.tjernlund@infinera.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v3] Unbreak av_malloc_max(0) API/ABI | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | warning | Make failed |
On Fri, 2020-10-16 at 01:38 +0200, Andreas Rheinhardt wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > Joakim Tjernlund: > > From https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D1095962&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=Lcc%2BcVTlLU1y6EqrMXwfXJ0enHYlIRTBJyGkQgQEviA%3D&reserved=0 > > ---------------------------- > > 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fthird_party%2Fffmpeg%2F%2B%2F73563&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=4%2BwE%2FMIcFSZlTdgzbVdbEBdYlO6Cdx%2Fh%2BLfjtrxCGec%3D&reserved=0 > > [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FFFmpeg%2FFFmpeg%2Fcommit%2F731c77589841&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=fuSKAPgqOiDsqwjl1m6P5IPF4a1K%2ByUK1c9e518aV6c%3D&reserved=0 > > --------------------------- > > > > Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older chromium etc. > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > --- > > > > v2: Cover the full API range 0-31 > > > > v3: Closer compat with < 4.3 ffmpeg > > > > libavutil/mem.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavutil/mem.c b/libavutil/mem.c > > index cfb6d8a..bd1fb85 100644 > > --- a/libavutil/mem.c > > +++ b/libavutil/mem.c > > @@ -71,6 +71,8 @@ void free(void *ptr); > > static size_t max_alloc_size= INT_MAX; > > > > void av_max_alloc(size_t max){ > > + if (max < 32) > > + max = SIZE_MAX - max; /* be compatible to older(< 4.3) versions */ > > max_alloc_size = max; > > } > > > > > For full compatibility it should be SIZE_MAX - 32 + max. > OK, v4 sent. > But why don't you go the way of fixing the broken apps? Because they are binary apps, in my case from Microsoft. Their MS Teams is based on a Chromium/Electron framework that(I hope) will be updated at some point. > > - Andreas
Joakim Tjernlund: > On Fri, 2020-10-16 at 01:38 +0200, Andreas Rheinhardt wrote: >> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >> >> >> Joakim Tjernlund: >>> From https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D1095962&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=Lcc%2BcVTlLU1y6EqrMXwfXJ0enHYlIRTBJyGkQgQEviA%3D&reserved=0 >>> ---------------------------- >>> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fthird_party%2Fffmpeg%2F%2B%2F73563&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=4%2BwE%2FMIcFSZlTdgzbVdbEBdYlO6Cdx%2Fh%2BLfjtrxCGec%3D&reserved=0 >>> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FFFmpeg%2FFFmpeg%2Fcommit%2F731c77589841&data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb1993f8740d849953d7908d871638074%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637384019459705602&sdata=fuSKAPgqOiDsqwjl1m6P5IPF4a1K%2ByUK1c9e518aV6c%3D&reserved=0 >>> --------------------------- >>> >>> Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older chromium etc. >>> >>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>> --- >>> >>> v2: Cover the full API range 0-31 >>> >>> v3: Closer compat with < 4.3 ffmpeg >>> >>> libavutil/mem.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavutil/mem.c b/libavutil/mem.c >>> index cfb6d8a..bd1fb85 100644 >>> --- a/libavutil/mem.c >>> +++ b/libavutil/mem.c >>> @@ -71,6 +71,8 @@ void free(void *ptr); >>> static size_t max_alloc_size= INT_MAX; >>> >>> void av_max_alloc(size_t max){ >>> + if (max < 32) >>> + max = SIZE_MAX - max; /* be compatible to older(< 4.3) versions */ >>> max_alloc_size = max; >>> } >>> >>> >> For full compatibility it should be SIZE_MAX - 32 + max. >> > OK, v4 sent. > >> But why don't you go the way of fixing the broken apps? > > Because they are binary apps, in my case from Microsoft. > Their MS Teams is based on a Chromium/Electron framework that(I hope) will > be updated at some point. > And have you already reported this issue to them? - Andreas
On Sat, 2020-10-17 at 19:52 +0200, Andreas Rheinhardt wrote: > Joakim Tjernlund: > > On Fri, 2020-10-16 at 01:38 +0200, Andreas Rheinhardt wrote: > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > Joakim Tjernlund: > > > > From https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.chromium.org%2Fp%2Fchromium%2Fissues%2Fdetail%3Fid%3D1095962&data=04%7C01%7Cjoakim.tjernlund%40infinera.com%7C96ae5fc7646d4c878d4508d872c57b85%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637385539798829478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=T7f337WCuCELJ6dn8imCFI%2FU6NQEnBeYQlbt0xV8Pvc%3D&reserved=0 > > > > ---------------------------- > > > > 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fthird_party%2Fffmpeg%2F%2B%2F73563&data=04%7C01%7Cjoakim.tjernlund%40infinera.com%7C96ae5fc7646d4c878d4508d872c57b85%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637385539798829478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=naVuSqIeUOCQqtm9D3SdOTT2FVoEkbqSAm7cXujILBM%3D&reserved=0 > > > > [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FFFmpeg%2FFFmpeg%2Fcommit%2F731c77589841&data=04%7C01%7Cjoakim.tjernlund%40infinera.com%7C96ae5fc7646d4c878d4508d872c57b85%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637385539798829478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rpeQH9PlbJlftXyfriVaMH%2BGtIkc6QNsls4KN%2B9Eeo0%3D&reserved=0 > > > > --------------------------- > > > > > > > > Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older chromium etc. > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > --- > > > > > > > > v2: Cover the full API range 0-31 > > > > > > > > v3: Closer compat with < 4.3 ffmpeg > > > > > > > > libavutil/mem.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/libavutil/mem.c b/libavutil/mem.c > > > > index cfb6d8a..bd1fb85 100644 > > > > --- a/libavutil/mem.c > > > > +++ b/libavutil/mem.c > > > > @@ -71,6 +71,8 @@ void free(void *ptr); > > > > static size_t max_alloc_size= INT_MAX; > > > > > > > > void av_max_alloc(size_t max){ > > > > + if (max < 32) > > > > + max = SIZE_MAX - max; /* be compatible to older(< 4.3) versions */ > > > > max_alloc_size = max; > > > > } > > > > > > > > > > > For full compatibility it should be SIZE_MAX - 32 + max. > > > > > OK, v4 sent. > > > > > But why don't you go the way of fixing the broken apps? > > > > Because they are binary apps, in my case from Microsoft. > > Their MS Teams is based on a Chromium/Electron framework that(I hope) will > > be updated at some point. > > > And have you already reported this issue to them? I tried that once earlier and didn't find howto. One would think MS would have some Teams presence on github. Turns out they do but not for reporting bugs on Teams itself. Anyhow, I got in touch with a MS dev. that may be able to help out. I do suspect this buried far down in electron framework and it will take time until all layers has been update and released. Now about the v4 patch, what is the verdict ? Jocke
diff --git a/libavutil/mem.c b/libavutil/mem.c index cfb6d8a..bd1fb85 100644 --- a/libavutil/mem.c +++ b/libavutil/mem.c @@ -71,6 +71,8 @@ void free(void *ptr); static size_t max_alloc_size= INT_MAX; void av_max_alloc(size_t max){ + if (max < 32) + max = SIZE_MAX - max; /* be compatible to older(< 4.3) versions */ max_alloc_size = max; }
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.tjernlund@infinera.com> --- v2: Cover the full API range 0-31 v3: Closer compat with < 4.3 ffmpeg libavutil/mem.c | 2 ++ 1 file changed, 2 insertions(+)