1433158 - Update libvpx to 1.7.0

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Tom Ritter [:tjr]]

This is a (semi-)automated bug making you aware that there is an available upgrade for an embedded third-party library. You can leave this bug open, and it will be updated if a newer version of the library becomes available. If you close it as WONTFIX, please indicate if you do not wish to receive any future bugs upon new releases of the library.

libvpx is currently at version 1.6.1 in mozilla-central, and the latest version of the library released is 1.7.0. 

I fetched the latest version of the library from https://chromium.googlesource.com/webm/libvpx/+refs.



The release notes say:

>  - Bug fixes:
>    A variety of fuzzing issues.

I skimmed through a few commits and found 


vp9_quantize_ssse3_x86_64: fix out of bounds write
https://chromium.googlesource.com/webm/libvpx/+/84a7263d4c124919718aca2a7eef1a218216917b

two crash fixes
https://chromium.googlesource.com/webm/libvpx/+/9f36419bf21a5922ffc32c289e09dd9fa0eb4eb2
https://chromium.googlesource.com/webm/libvpx/+/81d66e2cc6ca895051f51373b540a842b888df9e

There's probably more but I stopped after the out of bounds write.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Note that we already have bug 1426988 filed for one known issues in 1.6.1.

Comment 2

[johannkoenig]

The "two crash fixes" listed are for bug 1426988

Comment 3

[johannkoenig]

I just test updated android directly from 1.6.1 to 1.7.0.

The updated script needed --disable-avx512

Other than that, no changes. Let me know if anything weird crops up.

Comment 4

[Nils Ohlmeier [:drno]]

I suggest we use bug 1426988 to uplift only the sec patches to Beta (and potentially other branches) and land the update to libvpx 1.7.0 on 60 and let it ride the trains.

Comment 6

[Nils Ohlmeier [:drno]]

Just for the record: RyanVM convinced me in bug 1426988 to land bug 1426988 on central.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Per comment 4, this is only targeting 60+.

Comment 8

[Julien Cristau [:jcristau]]

I'm going to untrack this for 60 as the sec fix landed separately in bug 1426988.

Comment 9

[Frederik Braun [:freddy]]

Is updating libvpx still a security issue with bug 1426988 fixed?

Comment 10

[Tom Ritter [:tjr]]

(In reply to Frederik Braun [:freddyb] from comment #9)
> Is updating libvpx still a security issue with bug 1426988 fixed?

There's https://chromium.googlesource.com/webm/libvpx/+/84a7263d4c124919718aca2a7eef1a218216917b - I'm not sure if we're affected by it.

Comment 11

[Ralph Giles (:rillian)]

MediaRecoder doesn't support vp9 encoding, but it looks like we'd be vulnerable through WebRTC. The patch is one line and looks safe enough to uplift to 59 beta. I'll open a follow-up bug.

Comment 12

[Tom Ritter [:tjr]]

I used a new script to look through old vpx commits. 

It found:
45daecb4f73a47ab3236a29a3a48c52324cbf19a - vp8_decode_frame: fix oob read on truncated key frame
4ffdf60b85d2ad7f93ef451e4ec3be30ca797232 - described as a divide by zero, but links to a restricted bug
519fed01c2846ab9294543a3d2d65efaa51ec85b - described as an asan crash

I don't think we've investigated these.

84a7263d4c124919718aca2a7eef1a218216917b - out of bound write - Bug 1443865 FIXED

There were a lot (20+) commits about integer overflows.

Comment 13

[Frederik Braun [:freddy]]

With bug 1443865 and bug 1426988 fixed, maybe this one here is more of a maintenance bug and less of a security bug?

Comment 14

[Frederik Braun [:freddy]]

(In reply to Frederik Braun [:freddyb] from comment #13)
> With bug 1443865 and bug 1426988 fixed, maybe this one here is more of a
> maintenance bug and less of a security bug?

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Rillian has left Mozilla

Comment 16

[Nils Ohlmeier [:drno]]

(In reply to Frederik Braun [:freddyb] (Unavailable until August 20th) from comment #13)
> With bug 1443865 and bug 1426988 fixed, maybe this one here is more of a
> maintenance bug and less of a security bug?

Yes I think you are right that this mostly a maintenance bug now. It should be safe to open it up.

Comment 17

[Nils Ohlmeier [:drno]]

Dan, do you have enough cycles to take care of this update?

Comment 18

[Dan Minor [:dminor]]

Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de5846d12a033ec511a871e4f19fbcdac94db795

Comment 19

[Dan Minor [:dminor]]

Attachment: Bug 1433158 - Remove vp9_svc.patch; r=jya

Because webrtc.org updates are managed separately from libvpx updates, attempting
to apply this patch just leads to merge conflicts while doing a libvpx update.

Comment 20

[Dan Minor [:dminor]]

Attachment: Bug 1433158 - Update libvpx to 1.7.0; r=jya

Comment 21

[Dan Minor [:dminor]]

Attachment: Bug 1433158 - Disable avx512 in libvpx; r=jya

This disables building avx512 due to difficulty in getting a working set of
compiler flags across our supported platforms. Rather than carrying this patch
forward, we should revisit supporting avx512 when we do the next update.

Comment 22

[Jean-Yves Avenard [:jya]]

Comment on attachment 8998529 [details]
Bug 1433158 - Disable avx512 in libvpx; r=jya

Jean-Yves Avenard [:jya] has approved the revision.

Comment 23

[Jean-Yves Avenard [:jya]]

Comment on attachment 8998527 [details]
Bug 1433158 - Update libvpx to 1.7.0; r=jya

Jean-Yves Avenard [:jya] has approved the revision.

Comment 24

[Jean-Yves Avenard [:jya]]

Comment on attachment 8998526 [details]
Bug 1433158 - Remove vp9_svc.patch; r=jya

Jean-Yves Avenard [:jya] has approved the revision.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/607f7fbeaeb4
Remove vp9_svc.patch; r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/04a7c8e2ab6d
Update libvpx to 1.7.0; r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ffffe663eb9
Disable avx512 in libvpx; r=jya

Comment 26

[johannkoenig]

Re avx512:
https://chromium.googlesource.com/chromium/src/+/master/third_party/libvpx/generate_gni.sh#24

Comment 27

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/607f7fbeaeb4
https://hg.mozilla.org/mozilla-central/rev/04a7c8e2ab6d
https://hg.mozilla.org/mozilla-central/rev/6ffffe663eb9

Comment 28

[Daniel Holbert [:dholbert]]

Looks like this requires a CLOBBER file touch. mihir ran into this build error after updating his mozilla-central clone today:
> 0:02.99 make[4]: *** No rule to make target `$SRC/media/libvpx/libvpx/vp8/encoder/x86/quantize_ssse3.c', needed by `quantize_ssse3.o'.  Stop.

The quantize_ssse3.c source file was removed in this bug (specifically, in this gigantic update commit: https://hg.mozilla.org/mozilla-central/rev/04a7c8e2ab6d )

So, seems likely that our build system isn't automatically able to clean up after that file removal, so we need to bump CLOBBER to avoid other folks running into the same problem that mihir hit.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90a8b7d09a4b
followup: touch CLOBBER file to avoid bustage from libvpx update. rs=jya

Comment 30

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/90a8b7d09a4b

Comment 31

[Thomas Wisniewski [:twisniewski]]

Not that it matters much, but it seems that the patches here added a stray .orig file into the tree: media/libvpx/libvpx/vp9/vp9_cx_iface.c.orig

Comment 32

[Dan Minor [:dminor]]

(In reply to twisniewski from comment #31)
> Not that it matters much, but it seems that the patches here added a stray
> .orig file into the tree: media/libvpx/libvpx/vp9/vp9_cx_iface.c.orig

Oops. That will get removed by the next update. Not sure if it is worth removing it before then.