1114867 - [EME] Clear stack memory after device binding in GMPLoader

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

In bug 1101308 I discovered that the code to zero the memory on the stack down to the guard page was corrupting parts of the stack we were using and causing GMPs to crash randomly. I will remove that code in bug 1101308, but we need to reinstate a fixed copy of this code to ensure that no user-identifiable state is left behind after we've performed device binding.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 1: revert backout

This patch just reverts the prior backout. The actual fix is in the next patch, for easier review.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 2: manually inline RtlSecureZeroMemory

This basically inline code equivalent to the x86 case of _RtlSecureZeroMemory from winnt.h on my machine (Visual Studio 2013 Pro).

I didn't inline the x64 case using __stosb. I haven't tested x64.

I tested using __stosb in my 32bit build, and it seemed to work fine. I don't know why the MS forceinline definition of RtlSecureZeroMemory didn't use __stosb in the x86 case too, I fear there's a good reason, so I didn't want to risk using it myself.

I checked the assembly in an opt build on my machine, and the volatile loop was not optimized out.

Comment 3

[(Away)]

It would be nice to have a test for this, to make sure some future compiler doesn't mess with the loop... but I'm guessing it wouldn't be easy.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Presumably, gtests will start crashing again. I never saw them reported failing on our test machines, only locally however...

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cffdee8c23ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/e573fcf60968

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cffdee8c23ad
https://hg.mozilla.org/mozilla-central/rev/e573fcf60968

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Mass update firefox-status to track EME uplift.