759891 - Update libjpeg-turbo to 1.2.x branch r831 (or later)

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Justin Lebar (not reading bugmail)]

libjpeg-turbo 1.2.x r831 fixes a potential security vulnerability.  We should update ASAP.

Ryan, are you interested in doing this again?

Comment 1

[Ryan VanderMeulen [:RyanVM]]

I'm traveling until the end of next week. I can do it if nobody else does first.

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Part 1: Update MOZCHANGES file.

Comment 3

[Justin Lebar (not reading bugmail)]

Attachment: Part 2: Update the code.

I'll probably fold these two csets together when I check them in, but I thought it would be easier to review separately.

Comment 4

[Justin Lebar (not reading bugmail)]

The process I used to generate part 2 was:

 * Update my svn clone to r831
 * diff -r -U8 media/libjpeg ~/my/libjpeg-turbo/clone > ~/patch
 * Clean up ~/patch (remove spurious differences, e.g. because libjpeg-turbo has a different Makefile.in)

Comment 5

[Justin Lebar (not reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=26474b1d7fee

Comment 6

[Justin Lebar (not reading bugmail)]

Looks good on try; I'll land once I get r+ on the other patch.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

Review of attachment 630080 [details] [diff] [review]:
-----------------------------------------------------------------

Sure

Comment 8

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d10a38139eb8

Comment 9

[Justin Lebar (not reading bugmail)]

Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

[Approval Request Comment]
Security fix.  No string changes.

If we wanted a smaller patch, we could probably cherry-pick the fix from libjpeg-turbo, instead of upgrading wholesale.  But I think landing what we're landing in nightly onto branches makes sense, because that's what we're testing.

Happy to let this bake on trunk for a day or two, but nom'ing now so it's on everyone's radar.

Comment 10

[Joe Drew (not getting mail)]

I would much rather have a targeted patch.

Comment 11

[Justin Lebar (not reading bugmail)]

(In reply to Joe Drew (:JOEDREW!) from comment #10)
> I would much rather have a targeted patch.

My concern is just that we're going to test the full r831 on Nightly, so we'd be introducing an effectively new version of the jpeg decoder just for branches.

Maybe DRC could verify for us that cherry-picking the fix is safe.  I'll post a patch.

Comment 12

[Justin Lebar (not reading bugmail)]

> I'll post a patch.

...to bug 759802.

Comment 13

[DRC]

830->831 does not depend on any prior patches, so it should be safe to apply it without any of the other 1.2.1 patches.

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d10a38139eb8

Comment 15

[Daniel Veditz [:dveditz]]

Despite the approval requests here, do we want the smaller patch in bug 759802 instead? Definitely for ESR we'd want the small one, for Aurora we could go with this one, and Beta is a toss-up in my mind.

Comment 16

[Justin Lebar (not reading bugmail)]

I think taking the smaller patch on Beta and ESR and the larger patch on Aurora would be fine, but I'd like the release drivers' input, since that effectively adds a second libjpeg upgrade, which adds some risk.

Comment 17

[Alex Keybl [:akeybl]]

Comment on attachment 630080 [details] [diff] [review]
Part 2: Update the code.

Let's just take the patch in bug 759802 for all branches, and wait till FF16 to take the rest of the updates to libjpeg since there doesn't appear to be any significant user benefit at this time.