684882 - (CVE-2011-3653) Random video memory grabbed into WebGL cube map textures on Mac OS, including on 10.7.1, on Intel GPUs

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Claus Wahlers [:cwahlers]]

Attachment: firefox_6_0_1.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/535.1 (KHTML, like Gecko) Chrome/13.0.782.220 Safari/535.1

Steps to reproduce:

I loaded this WebGL demo in Firefox 6.0.1:
http://mrdoob.github.com/three.js/examples/webgl_materials_cars_anaglyph.html


Actual results:

The background cube texture does not render correctly. What i see is randomly garbled artefacts of parts of my screen. See attached screenshots.

The effect is very similar to what has already been reported in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=631258 - just that i run a different OS version and a different graphics card.

Note that i can reproduce this issue (even after rebooting my machine), and that Chrome 13 shows the exact same problem (suggesting that it is a low level problem, possibly with the graphics card drivers).

Some info on my hardware:

Hardware Overview:
  Model Name:	MacBook Pro
  Model Identifier:	MacBookPro8,1
  Processor Name:	Intel Core i5
  Processor Speed:	2.3 GHz
  Number of Processors:	1
  Total Number of Cores:	2
  L2 Cache (per Core):	256 KB
  L3 Cache:	3 MB
  Memory:	4 GB
  Boot ROM Version:	MBP81.0047.B0E
  SMC Version (system):	1.68f96

System Software Overview:
  System Version:	Mac OS X 10.6.8 (10K549)
  Kernel Version:	Darwin 10.8.0
  Boot Volume:	Macintosh HD
  Boot Mode:	Normal
  Secure Virtual Memory:	Enabled
  64-bit Kernel and Extensions:	Yes

Intel HD Graphics 3000:
  Chipset Model:	Intel HD Graphics 3000
  Type:	GPU
  Bus:	Built-In
  VRAM (Total):	384 MB
  Vendor:	Intel (0x8086)
  Device ID:	0x0126
  Revision ID:	0x0009
  Displays:
Color LCD:
  Resolution:	1280 x 800
  Pixel Depth:	32-Bit Color (ARGB8888)
  Main Display:	Yes
  Mirror:	Off
  Online:	Yes
  Built-In:	Yes
Display Connector:
  Status:	No Display Connected


Expected results:

A nice cube texture should be shown, without artefacts.

Comment 1

[Claus Wahlers [:cwahlers]]

Attachment: Same demo run in Chrome, shows same problem

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

@ Daniel: couldn't we un-hide this bug? Bug 631258 is already public, and the original reporter already blogged about it: http://wahlers.com.br/claus/blog/talking-about-webgl-and-security

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Claus Wahlers from comment #0)
> Note that i can reproduce this issue (even after rebooting my machine)

Sorry to stubbornly asking for confirming this multiple times, but this is really very important:

Are you sure you can reproduce this IMMEDIATELY after rebooting your machine, without first letting it sleep, without letting any screensaver run, etc? I would also like to know if you can reproduce this without running any other application than just Firefox and Mac OS itself (and the Finder) since you rebooted?

Comment 4

[Claus Wahlers [:cwahlers]]

Yes, immediately after rebooting, no sleep, no screensaver.

Although my system is set up to launch some applications on login:
- GrowlHelperApp
- GrowlMenu
- Dropbox
- Echofon
- Skype
- iTunesHelper
- iChat
- TotalFinder
- Google Chrome

Comment 5

[Zhenyao Mo]

Our of curiosity, is the behavior exactly the same in Chrome?

Comment 6

[Claus Wahlers [:cwahlers]]

Yes, see attached screenshot.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Two things that would be nice to know:
1. can any of us developers here reproduce this issue?
2. can this issue be reproduced on Mac OS 10.7? AIUI 10.7 has GL-accelerated 2D graphics so for example Safari would in principle have the same bug exposed via Canvas 2D.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

@ Claus: you said this is a MacBook Pro with Intel graphics. Many MacBook Pro computers have a discrete GPU, and my understanding is that many have dual GPUs (Intel and discrete). Do you have a discrete (ATI or NVIDIA) GPU in this machine? Sometimes it's hard to know which GPU is used.

Comment 9

[Claus Wahlers [:cwahlers]]

I don't know. How can i find out?

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Apparently the gfxCardStatus tool can tell you that and, in the case of a dual GPU machine, allow you to force usage of a specific GPU:

http://codykrieger.com/gfxCardStatus

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

Another thing you can do is, in Firefox, go to the special about:support page. Scroll down to Graphics, please paste the Graphics section here.

Comment 12

[Claus Wahlers [:cwahlers]]

about:support Graphics:
Adapter Description: 0x24300,0x20400
WebGL Renderer: Intel Inc. -- Intel HD Graphics 3000 OpenGL Engine -- 2.1 APPLE-1.6.36
GPU Accelerated Windows: 1/1 OpenGL

I installed gfxCardStatus, it has three options:
- Integrated Only
- Discrete Only
- Dynamic Switching
All options are enabled, but i can't seem to select the Discrete Only and Dynamic Switching options.

Please also see the System Profiler dumps i originally posted with this bug.

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

Thanks. All that is pretty conclusive that there is only Intel integrated graphics here.

> Adapter Description: 0x24300,0x20400

0x24300 means recent Intel GPU, 0x20400 is the software rendering fallback

Firefox has a bug making it use the discrete GPU when there is one: bug 646043. The fact that about:support plus gfxCardStatus agree that you have Intel only is quite conclusive.

Comment 14

[Kenneth Russell]

I've been able to reproduce incorrect rendering of the cube map texture in the above sample on a MacBook Pro with a dual NVIDIA/Intel GPU, using gfxCardStatus to force the use of the integrated GPU. I was testing with a Chromium, not a Firefox, build, which indicates a driver bug. I don't see random contents of VRAM in the textures, but some black areas and stretched or skewed rendering of the cube map on two of the faces.

I recall a similar rendering artifact on Mac OS some time ago where calling glGenerateMipmapsEXT would sometimes corrupt textures, specifically if the texture object's minification filter didn't need mipmaps at the time the call was made. In Chromium a workaround was added to temporarily change the filtering mode of the texture. I'll need to review the code to see if we implemented a similar workaround for cube map textures and if that seems to be the problem here.

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Kenneth Russell from comment #14)
> I recall a similar rendering artifact on Mac OS some time ago where calling
> glGenerateMipmapsEXT would sometimes corrupt textures, specifically if the
> texture object's minification filter didn't need mipmaps at the time the
> call was made. In Chromium a workaround was added to temporarily change the
> filtering mode of the texture. I'll need to review the code to see if we
> implemented a similar workaround for cube map textures and if that seems to
> be the problem here.

OK, that's very interesting. I was going to ask what happens if the wrap modes are CLAMP_TO_EDGE and the texture is NPOT, but Macs have desktop OpenGL with full support for NPOT textures, so there is no problem.

If that still doesn't work then a different class of work-arounds might be to avoid calling glGenerateMipmaps altogether and implement ourselves...  using the texture to paint a quad on a framebuffer backed by a texture of half the size, then on another with 1/4 the size, etc... This wouldn't be the first crazy Mac GL workaround we have to do.

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: work around glGenerateMipmap bug

This implements the idea mentioned by Ken, that this is a glGenerateMipmap bug that can be worked around by always having a min filter requiring a mipmap when this function is called.

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: work around glGenerateMipmap bug

forgot to save a file.

Comment 18

[Benoit Jacob [:bjacob] (mostly away)]

Try push:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=b4ed5844c2f7

When the builds are ready (in 2-3 hours) they'll be available here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-b4ed5844c2f7

Please test them!

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 559525 [details] [diff] [review]
work around glGenerateMipmap bug

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

::: content/canvas/src/WebGLContextGL.cpp
@@ +1768,5 @@
> +        gl->fTexParameteri(target, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR_MIPMAP_LINEAR);
> +        gl->fGenerateMipmap(target);
> +        gl->fTexParameteri(target, LOCAL_GL_TEXTURE_MIN_FILTER, tex->MinFilter());
> +    }
> +#else

Please mention the Apple bug report here.

Comment 20

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/db67bdba5ca8

Comment 21

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 559525 [details] [diff] [review]
work around glGenerateMipmap bug

Please approve. Really innocuous patch with a high probability of fixing this really bad bug.

Ignore the tryserver build, it seems to have choked but it was based of mozilla-inbound code and the failure seems unrelated to my patch so I'm ignoring it.

Please test tomorrow's Nightly build.

Comment 22

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/db67bdba5ca8

Comment 24

[Kenneth Russell]

Unfortunately, the corrupted rendering persists with the nightly dated 9.0a1 (2011-09-10), on a MacBook Pro with dual Intel HD Graphics (Device ID 0x0046) and NVIDIA GeForce GT 330M. I quit all applications, used gfxCardStatus to force use of the integrated GPU, launched the nightly and navigated to the above sample.

Does the build I used contain this fix?

Comment 25

[Kenneth Russell]

I forgot to mention that the machine above is running 10.7.1 (11B26).

Comment 26

[Benoit Jacob [:bjacob] (mostly away)]

Thanks a lot again for checking this (in addition to thanks for the glGenerateMipmap hint!)

Just for the record the way to check this for sure is got to about:buildconfig, and check the changeset id and compare in the hg log to the id for this fix, which is db67bdba5ca8.

Looking in the nightlies archive, it seems that the changeset ID is 06b2977afb85:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-09-10-03-08-51-mozilla-central/firefox-9.0a1.en-US.mac.txt

I confirm that it does contain this patch.

So that means that the corruption you're seeing is not fixed by this patch.

I would still like to know if the corruption originally reported in this bug is fixed by this patch. Indeed the one you're seeing seems a bit different:

(In reply to Kenneth Russell from comment #14)
> I don't see
> random contents of VRAM in the textures, but some black areas and stretched
> or skewed rendering of the cube map on two of the faces.

I would still like to know if the other-windows-grabbed-into-WebGL-textures bug originally reported here is fixed by this bug.

Claus?

Comment 27

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 559525 [details] [diff] [review]
work around glGenerateMipmap bug

OK, this work-around is not successful. Cancelling approval requests. Will try variant patch where glGenerateMipmap is completely emulated.

Comment 28

[Benoit Jacob [:bjacob] (mostly away)]

Note: http://wahlers.com.br/claus/blog/talking-about-webgl-and-security/#comment-81653

Comment 29

[Benoit Jacob [:bjacob] (mostly away)]

Similar issue been discussed here, with a GL trace:
   http://lists.apple.com/archives/mac-opengl/2011/Jul/msg00011.html

The reply says that Apple is aware of the issue and it's happening in certain OOM conditions...

   -> when the bug happens, do you get a OUT_OF_MEMORY WebGL error?

This other discussion,
    http://www.opengl.org/discussion_boards/ubbthreads.php?ubb=showflat&Number=273603
recommends the exact same workaround as I implemented... it seems to be working for them.

Comment 30

[Benoit Jacob [:bjacob] (mostly away)]

Two new tryserver builds. I appreciate a lot if you can check them again. I understand it's time-consuming but we desperately need to work around this bug. These are my last attempts before I completely give up on calling glGenerateMipmap and emulate it completely, but that would be a long patch to write and would hurt performance.

Please try this one first:

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-948b79ee11f3/try-macosx64/

This patch preallocates all images in all mipmap levels with zero data, so that glGenerateMipmap shouldn't have to allocate any buffer. The hope is that that will dodge the bug.

If this doesn't work, then this second patch:

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e80e7435e38f/

Goes back to the original approach discussed above, since it seems to be working for people, and adds a glFinish() call after glGenerateMipmap before we reset the texture minification filter. The idea is to ensure that glGenerateMipmap does get (as a result of another bug) postponed until after the minification filter is restored. Of course in principle this should not be needed but since we're on the topic of bugs...

Comment 31

[Kenneth Russell]

I tried both of the above builds but unfortunately they still exhibit the problem.

Before you write a huge patch reimplementing glGenerateMipmaps, I think that we should try modifying the app locally to understand what operations it's doing. I'm not even sure it's trying to use a mipmapped cube map at all. If we can figure out some small changes to make the app work, we will understand better what workaround, if any, we could do in the WebGL implementation.

Do you have hardware in house that reproduces the problem? I will try to start looking at the demo's source code later today or tomorrow.

Comment 32

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Kenneth Russell from comment #31)
> Before you write a huge patch reimplementing glGenerateMipmaps, I think that
> we should try modifying the app locally to understand what operations it's
> doing. I'm not even sure it's trying to use a mipmapped cube map at all.

Here's a trace of Firefox running this demo, recorded in APItrace, on my linux machine:

2598 glGenTextures(n = 1, textures = &5)
2600 glActiveTexture(texture = GL_TEXTURE1)
2602 glBindTexture(target = GL_TEXTURE_CUBE_MAP, texture = 5)
2603 glTexParameteri(target = GL_TEXTURE_CUBE_MAP, pname = GL_TEXTURE_WRAP_R, param = GL_CLAMP_TO_EDGE)
2605 glGetError() = GL_NO_ERROR
2606 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_POSITIVE_X, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2607 glGetError() = GL_NO_ERROR
2609 glGetError() = GL_NO_ERROR
2610 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_NEGATIVE_X, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2611 glGetError() = GL_NO_ERROR
2613 glGetError() = GL_NO_ERROR
2614 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_POSITIVE_Y, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2615 glGetError() = GL_NO_ERROR
2617 glGetError() = GL_NO_ERROR
2618 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_NEGATIVE_Y, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2619 glGetError() = GL_NO_ERROR
2621 glGetError() = GL_NO_ERROR
2622 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_POSITIVE_Z, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2623 glGetError() = GL_NO_ERROR
2625 glGetError() = GL_NO_ERROR
2626 glTexImage2D(target = GL_TEXTURE_CUBE_MAP_NEGATIVE_Z, level = 0, internalformat = GL_RGBA, width = 1024, height = 1024, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4194304))
2627 glGetError() = GL_NO_ERROR
2629 glTexParameteri(target = GL_TEXTURE_CUBE_MAP, pname = GL_TEXTURE_WRAP_S, param = GL_CLAMP_TO_EDGE)
2631 glTexParameteri(target = GL_TEXTURE_CUBE_MAP, pname = GL_TEXTURE_WRAP_T, param = GL_CLAMP_TO_EDGE)
2633 glTexParameteri(target = GL_TEXTURE_CUBE_MAP, pname = GL_TEXTURE_MAG_FILTER, param = GL_LINEAR)
2635 glTexParameteri(target = GL_TEXTURE_CUBE_MAP, pname = GL_TEXTURE_MIN_FILTER, param = GL_LINEAR_MIPMAP_LINEAR)
2637 glGenerateMipmap(target = GL_TEXTURE_CUBE_MAP)

I have only removed glXGetCurrentContext calls, otherwise these lines are contiguous in the trace.

So it is calling glGenerateMipmap on a cube map texture. Interestingly the min filter is GL_LINEAR_MIPMAP_LINEAR defeating our theory that this bug would be specific to min filters not requiring a mipmap.


> If
> we can figure out some small changes to make the app work, we will
> understand better what workaround, if any, we could do in the WebGL
> implementation.

The active texture unit is the 2nd one:

2600 glActiveTexture(texture = GL_TEXTURE1)

Do you think this might remotely be worth setting to GL_TEXTURE0 to see if the bug persists?

> 
> Do you have hardware in house that reproduces the problem? I will try to
> start looking at the demo's source code later today or tomorrow.

No, we've not been able to reproduce the bug at Mozilla.

Comment 33

[Benoit Jacob [:bjacob] (mostly away)]

We have a winner - Benoit (not me) is able to reproduce on his Mac using GfxCardStatus.

Comment 34

[Mark Callow]

Two things could use clarification.

Is the other Benoit able to reproduce the bug described by the OP or the bug described in comment #14.

The bug title claims the bug affects all GPUs yet GfxCardStatus was needed to reproduce by forcing selection of a specific GPU. Seems like a contradiction.

Comment 35

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Mark Callow from comment #34)
> Two things could use clarification.
> 
> Is the other Benoit able to reproduce the bug described by the OP or the bug
> described in comment #14.

The original bug. I saw his screen and it did have a corrupt texture with contents from his desktop.

> 
> The bug title claims the bug affects all GPUs yet GfxCardStatus was needed
> to reproduce by forcing selection of a specific GPU. Seems like a
> contradiction.

Benoit could only reproduce with his integrated Intel GPU, set with gfxCardStatus. He could not reproduce with his ATI card.

Comment 36

[Benoit Jacob [:bjacob] (mostly away)]

It seems that the only report we've seen of this bug on a non-intel GPU was bug 631258, however that was on Mac OS 10.5. It seems that on recent Mac OS versions this bug can only be reproduced on Intel GPUs.

Updated steps to reproduce:
 1. use gfxCardStatus to force usage of the Intel GPU
 2. In Firefox or Chrome, go to
      http://people.mozilla.org/~bjacob/webgltexturecubemap.html
    or
      http://people.mozilla.org/~bjacob/webgltexturecubemap-blank.html

These pages are much simpler than the link in comment 0. All they do is a cube map with a mipmap.

Expected results:

* webgltexturecubemap.html shows see a triangle painted with a green/black cube map
* webgltexturecubemap-blank.html shows a white triangle (blank white cube map)

Actual results:

Rendering shows random texture memory from other applications blended into the texture.

Comment 37

[Benoit Jacob [:bjacob] (mostly away)]

Conversation with Ken and Daniel-from-Transgaming today, 2 interesting facts:
 * this glGenerateMipmap bug is only a consequence of a more far-reaching bug: illegal video memory is accessed when rendering to a face of a cube map. So we also need a patch to prevent attaching a face of a cube map to a framebuffer.
    -> this kills any hope of a work-around using the GPU to generate the mipmap.
 * there's agreement that there seem to have been 2 separate bugs:
     1. bug 631258 affected all GPUs, and all texture types, and seemed to be affected by texture minification filter, and seems to have been fixed in Mac OS 10.6+
     2. the present bug seems to be Intel-only and limited to cube maps, and unaffected by texture minification filter (as said above, it is purely about rendering to a cube map).

Since the present bug seems limited to cube maps, mipmapping only needs to be disabled for cube maps, which is more acceptable.

Comment 38

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: work around cube map bug

This patch does 3 things on Macs with Intel GPUs:
 * prevents attaching a cube map to a framebuffer.
     Test case: http://codeflow.org/webgl/irradiance/
 * skips glGenerateMipmaps on cube maps
 * since cube maps are now missing their mipmaps, prevents setting on them a minification filter that requires a mipmap, but only from OpenGL's point of view, not from WebGL's.

Note the Intel check is performed on GL context creation. You told me not to worry about that, that you would handle it once GPUs start changing under our feet... thanks :-)

Comment 39

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 561683 [details] [diff] [review]
work around cube map bug

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

::: content/canvas/src/WebGLContextGL.cpp
@@ +1700,3 @@
>          return ErrorInvalidOperation("framebufferTexture2D: cannot modify framebuffer 0");
> +
> +    if (textarget != LOCAL_GL_TEXTURE_2D && WorkAroundCubeMapBug684882()) {

I'd prefer textarget == LOCAL_GL_TEXTURE_CUBE_MAP

@@ +2530,5 @@
> +    
> +    WebGLint intParamForGL = intParam;
> +    WebGLfloat floatParamForGL = floatParam;
> +    
> +    if (WorkAroundCubeMapBug684882()) {

This stuff could use a summary of why it's needed.

Comment 40

[chris hofmann]

Seems like we need:

1) some clarity about if the current set of patches are a complete and comprehensive set of fixes.

2) get the patches landed on nightly and test

3) think about if we need to try and get the patches into fx7 or prepare for a chemspill if this gets widespread discovery.   fact that its observable on public test cases and blog post could lead to wider discovery.

Comment 41

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #39)
> Comment on attachment 561683 [details] [diff] [review] [diff] [details] [review]
> work around cube map bug
> 
> Review of attachment 561683 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: content/canvas/src/WebGLContextGL.cpp
> @@ +1700,3 @@
> >          return ErrorInvalidOperation("framebufferTexture2D: cannot modify framebuffer 0");
> > +
> > +    if (textarget != LOCAL_GL_TEXTURE_2D && WorkAroundCubeMapBug684882()) {

Also, this bug seems fixed in 10.7.2 so we should only block on a version less than 10.7.2

Comment 42

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #39)
> Comment on attachment 561683 [details] [diff] [review] [diff] [details] [review]
> work around cube map bug
> 
> Review of attachment 561683 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: content/canvas/src/WebGLContextGL.cpp
> @@ +1700,3 @@
> >          return ErrorInvalidOperation("framebufferTexture2D: cannot modify framebuffer 0");
> > +
> > +    if (textarget != LOCAL_GL_TEXTURE_2D && WorkAroundCubeMapBug684882()) {
> 
> I'd prefer textarget == LOCAL_GL_TEXTURE_CUBE_MAP

Not possible: the textarget parameter here indicates the precise face in case of a cube map, e.g. LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_X, so the if() would be a lot heavier this way.


> 
> @@ +2530,5 @@
> > +    
> > +    WebGLint intParamForGL = intParam;
> > +    WebGLfloat floatParamForGL = floatParam;
> > +    
> > +    if (WorkAroundCubeMapBug684882()) {
> 
> This stuff could use a summary of why it's needed.

Summary is given once in the WorkAroundCubeMapBug684882 function itself. I thought it was not worth repeating it everytime it's used.

Comment 43

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #41)
> Also, this bug seems fixed in 10.7.2 so we should only block on a version
> less than 10.7.2

OK, I also Benoit Girard's report today that it seems fixed on 10.7.2. Will do as a follow-up bug if you want. It's nontrivial to me how quickly we want to rely on that, since this bug never was reproducible on _all_ Mac/Intel machines, so it's not clear how conclusive a test done on one machine can be.

Comment 44

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to chris hofmann from comment #40)
> Seems like we need:
> 
> 1) some clarity about if the current set of patches are a complete and
> comprehensive set of fixes.

Yes, the current patch, https://bugzilla.mozilla.org/attachment.cgi?id=561683 , is a full fix by itself for all the issues discussed here. It overwrites the previous patch, which turned out to not fix the issue.

> 
> 2) get the patches landed on nightly and test

On tryserver at the moment, will report to this bug once it's built. Plan to land today on m-c.

> 3) think about if we need to try and get the patches into fx7 or prepare for
> a chemspill if this gets widespread discovery.

Now that we know what the issue is, it's clear that it is quite easy to run into if you have a Mac with only Intel graphics (Macbook Air) and have not yet upgraded to Mac OS 10.7.2.

If we can take this as a last minute fix in Firefox 7, then great (but I would be surprised if we could do that this late). If we can land this on the release branch so it will make it into 7.0.1 if there is a 7.0.1, then great. What I don't know is if this issue is by itself severe enough to warrant a chemspill or generally panic about it.

A Chrome developer told me today that:
 1) Chrome is NOT going to respond to this by blacklisting WebGL on Mac/Intel. Instead they will do a work-around like us.
 2) Their patch will be very similar to ours, and has yet to be written.

Comment 45

[Jeff Muizelaar [:jrmuizel]]

(In reply to Benoit Jacob [:bjacob] from comment #42)
> Summary is given once in the WorkAroundCubeMapBug684882 function itself. I
> thought it was not worth repeating it everytime it's used.

I more mean what does the code below do? and how it is related to cube maps being broken?

Comment 46

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #45)
> (In reply to Benoit Jacob [:bjacob] from comment #42)
> > Summary is given once in the WorkAroundCubeMapBug684882 function itself. I
> > thought it was not worth repeating it everytime it's used.
> 
> I more mean what does the code below do? and how it is related to cube maps
> being broken?

Oh OK. Indeed. Will add a comment there.

Comment 47

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/b4fd4fd0dc3f

Comment 48

[Benoit Jacob [:bjacob] (mostly away)]

Claus tells me that he can still reproduce the bug... I wonder if something went wrong in the Intel vendor check. Pushed to try a patch that always does the workaround on Mac, not just on Intel GPUs. Tryserver will report to this bug.

Comment 49

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: do it on all GPUs, not just Intel

Holy smokes, this makes us pass the texture-npot.html test, which was so far failing but intermittently passing!

There was no way that I could guess that this test failure was related to a security bug!

Comment 50

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 562748 [details] [diff] [review]
do it on all GPUs, not just Intel

Doesn't this break drawing to cube maps on all of OS X?

Comment 51

[Benoit Jacob [:bjacob] (mostly away)]

It does. But if it's confirmed that 10.7.2 fixes the whole bug, we'll be able to limit that to 10.7.1 and older versions.

Comment 52

[JP Rosevear [:jpr]]

Comment on attachment 561683 [details] [diff] [review]
work around cube map bug

Not taking this until sure what the fix is, re-nom when ready.

Comment 53

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: blacklist WebGL on Intel GPUs, on Mac OS < 10.7.2

I would still like a confirmation on other machines that 10.7.2 fixes it.

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=7f6ce4b416b3
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-7f6ce4b416b3

Comment 54

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: blacklist WebGL on Intel GPUs, on Mac OS < 10.7.2, quick&dirty version for firefox 8

The first patch above relied on Jeff's GfxInfo.mm improvements that just landed on inbound.

If we want something less intrusive to ship in Firefox 8 (or even in a 7.0.1...) this new patch is it. The problem is that without Jeff's new GfxInfo.mm patches, on a dual-GPU Mac, GfxInfo doesn't really know which GPU we're using. The quick solution is to implement that in WebGLContext initialization once we already have a GL context. It's not great as it means we still pay the price of waiting for a GL context to be created before we decide not to allow WebGL, but it's not too terrible and is the best I can do with a non-intrusive patch.

Comment 55

[Cedric Vivier [:cedricv]]

Is this issue still reproducible with cube map textures smaller than 1024x1024?
Say 512x512?

(I don't have a Mac handy)

Comment 56

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: blacklist WebGL on Intel GPUs, on Mac OS < 10.7.2, quick&dirty version for firefox 8

Was missing a Mac #include.

Comment 57

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Cedric Vivier [cedricv] from comment #55)
> Is this issue still reproducible with cube map textures smaller than
> 1024x1024?
> Say 512x512?
> 
> (I don't have a Mac handy)

Actually, I tried today, and the answer seemed to be 'no'. At least for 512x512 and 256x256 I couldn't reproduce. But I didn't quite try hard enough to be sure.

Comment 58

[Cedric Vivier [:cedricv]]

(In reply to Benoit Jacob [:bjacob] from comment #57)
> (In reply to Cedric Vivier [cedricv] from comment #55)
> > Is this issue still reproducible with cube map textures smaller than
> > 1024x1024?
> 
> Actually, I tried today, and the answer seemed to be 'no'. At least for
> 512x512 and 256x256 I couldn't reproduce. But I didn't quite try hard enough
> to be sure.

Interesting.
Indeed I've read reports than the returned MAX_CUBE_MAP_TEXTURE_SIZE is too optimistic on OSX/IntelGPU and is actually between two and three times smaller than reported (what is the returned value on your test setup?)

If we can confirm it is safe, it would make sense imho to reduce MAX_CUBE_MAP_TEXTURE_SIZE on our side (or perhaps force the minimum profile [Bug 686732]) - which would be less disruptive than disabling WebGL completely.

Comment 59

[Jeff Muizelaar [:jrmuizel]]

(In reply to Cedric Vivier [cedricv] from comment #58)
> (In reply to Benoit Jacob [:bjacob] from comment #57)
> > (In reply to Cedric Vivier [cedricv] from comment #55)
> > > Is this issue still reproducible with cube map textures smaller than
> > > 1024x1024?
> > 
> > Actually, I tried today, and the answer seemed to be 'no'. At least for
> > 512x512 and 256x256 I couldn't reproduce. But I didn't quite try hard enough
> > to be sure.
> 
> Interesting.
> Indeed I've read reports than the returned MAX_CUBE_MAP_TEXTURE_SIZE is too
> optimistic on OSX/IntelGPU and is actually between two and three times
> smaller than reported (what is the returned value on your test setup?)
> 
> If we can confirm it is safe, it would make sense imho to reduce
> MAX_CUBE_MAP_TEXTURE_SIZE on our side (or perhaps force the minimum profile
> [Bug 686732]) - which would be less disruptive than disabling WebGL
> completely.

On my machine I get MAX_CUBE_MAP_TEXTURE_SIZE = 8192

Comment 60

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #59)
> (In reply to Cedric Vivier [cedricv] from comment #58)
> > If we can confirm it is safe, it would make sense imho to reduce
> > MAX_CUBE_MAP_TEXTURE_SIZE on our side (or perhaps force the minimum profile
> > [Bug 686732]) - which would be less disruptive than disabling WebGL
> > completely.
> 
> On my machine I get MAX_CUBE_MAP_TEXTURE_SIZE = 8192

And yes, I think that if we can find a size under which things work we should definitely go with that approach.

Comment 61

[Jeff Muizelaar [:jrmuizel]]

(In reply to Cedric Vivier [cedricv] from comment #58)
> Indeed I've read reports than the returned MAX_CUBE_MAP_TEXTURE_SIZE is too
> optimistic on OSX/IntelGPU and is actually between two and three times
> smaller than reported (what is the returned value on your test setup?)

Do you have links to these reports?

Comment 62

[Jeff Muizelaar [:jrmuizel]]

I tried to find the smallest cube map size with CLAMP_TO_EDGE that didn't have the problem and found the value to be 894

Comment 63

[Benoit Jacob [:bjacob] (mostly away)]

I confirm that the cube map faces in the original testcase for this bug are of size 1024x1024:
http://mrdoob.github.com/three.js/examples/textures/cube/Bridge2/posx.jpg

Comment 64

[Benoit Jacob [:bjacob] (mostly away)]

New testcase for 512x512 size:
http://people.mozilla.org/~bjacob/webgltexturecubemap-flat-512.html

please give it a try. if nobody can get corruption in this testcase then it should be safe to just limit cube map size to 512...

Comment 65

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: limit cube map size to 512

Comment 66

[Claus Wahlers [:cwahlers]]

(In reply to Benoit Jacob [:bjacob] from comment #64)
> New testcase for 512x512 size:
> http://people.mozilla.org/~bjacob/webgltexturecubemap-flat-512.html
> 
> please give it a try. if nobody can get corruption in this testcase then it
> should be safe to just limit cube map size to 512...

FF 6.0.2, FF Nightly 2011/09/26 and latest Chrome all show a black square on my system. OSX 10.6.8, Intel gfx.

Comment 67

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/c561909d40af

Comment 68

[Benoit Jacob [:bjacob] (mostly away)]

Note that I've also backed out the two previous patches (the failed work-arounds). Likewise we need to back them out from Aurora.

Comment 69

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 563299 [details]
limit cube map size to 512

Requesting approval for aurora, beta, and even release in case there's a 7.0.2.

Comment 70

[Mark Callow]

I do not think marking this "RESOLVED FIXED" is appropriate. You have a workaround that fixes the security problem but the underlying problem in the graphics driver remains and the workaround breaks app's that fail to check and accommodate the max size of a cube map face. You need an open tracking bug to remind you to remove the workaround when the OpenGL driver is fixed.

Comment 71

[Benoit Jacob [:bjacob] (mostly away)]

You're right about the need for a follow-up bug. I filed Bug 690738.

Comment 72

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 563299 [details]
limit cube map size to 512

This issue was discussed during today's WebGL conference call.

Ken reported that he could still reproduce the bug, using the original testcase (mrdoob's car demo) on Mac OS 10.7.2. So we can forget about the OS version check for now.

I synced with other browser vendors about the timeframe to ship a workaround. There was no objection against us releasing a workaround whenever we want, the point being that this issue has already been blogged about publicly. However, I think, and I replied, that once a Firefox version mentions it among the security fixes, this issue will get a lot more attention than it has so far, so I would still say we need to be considerate about the timeframe to avoid hurting other browser vendors and the WebGL platform in general. Shipping this workaround in Firefox 8 seems like the correct compromise. It's a predictable date for other browser vendors, as opposed to a 7.0.2 release which could ship at any time.

-> please approve for beta and aurora

Comment 73

[Benoit Jacob [:bjacob] (mostly away)]

...or if you really want to ship it in a 7.0.x release, please give us a precise timeframe estimate so we can share it with other browser vendors as soon as possible.

Comment 74

[Claus Wahlers [:cwahlers]]

FTR, i just retested and can confirm that the current nightly fixes the bug on my system (Mac OS X 10.6.8, Intel GFX). The mrdoob demo shows a black cube map. Great job!

And please accept my apologies for blogging the issue. I was acting as a slightly creeped out user, and not as the responsible developer that i am supposed to be. Next time i know better.

Thanks guys, great job!

Comment 75

[Benoit Jacob [:bjacob] (mostly away)]

No problem. Actually it seems that some people were already aware of this bug:
1. http://lists.apple.com/archives/mac-opengl/2011/Jul/msg00011.html
2. Flash 11's Stage3D limits cube maps to 512 pixels on all platforms.

Comment 76

[Claus Wahlers [:cwahlers]]

(In reply to Benoit Jacob [:bjacob] from comment #75)
> 2. Flash 11's Stage3D limits cube maps to 512 pixels on all platforms.

Just ran into this:
http://dl.dropbox.com/u/635275/Screen%20shot%202011-10-06%20at%206.11.06%20PM.png
(Stage3D demo running on Flash Player 11)

Comment 77

[Benoit Jacob [:bjacob] (mostly away)]

Landed on Beta (Firefox 8) and Aurora (Firefox 9).
http://hg.mozilla.org/releases/mozilla-aurora/rev/5cbbdef702d3
http://hg.mozilla.org/releases/mozilla-beta/rev/154e5af0dddd

Comment 78

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Claus Wahlers from comment #76)
> (In reply to Benoit Jacob [:bjacob] from comment #75)
> > 2. Flash 11's Stage3D limits cube maps to 512 pixels on all platforms.
> 
> Just ran into this:
> http://dl.dropbox.com/u/635275/Screen%20shot%202011-10-06%20at%206.11.
> 06%20PM.png
> (Stage3D demo running on Flash Player 11)

Indeed I was wrong, Stage3D doesn't limit the max texture size to 512 but to 1024, that's why they expose the bug.
http://help.adobe.com/en_US/FlashPlatform/beta/reference/actionscript/3/flash/display3D/Context3D.html#createCubeTexture%28%29

Comment 79

[Claus Wahlers [:cwahlers]]

(In reply to Benoit Jacob [:bjacob] from comment #78)
> Indeed I was wrong, Stage3D doesn't limit the max texture size to 512 but to
> 1024, that's why they expose the bug.
> http://help.adobe.com/en_US/FlashPlatform/beta/reference/actionscript/3/
> flash/display3D/Context3D.html#createCubeTexture%28%29

Thanks. I already made Adobe aware of the problem.

Comment 80

[u279076]

qa+ for verification in Firefox 9 with the test in comment 0.

Comment 82

[Benoit Jacob [:bjacob] (mostly away)]

Christian Legnitto [:LegNeato] 2011-11-08 17:04:54 PST
> Target Milestone: mozilla9 ➔ mozilla10

I don't understand that: this bug was essentially fixed in Firefox 8. There seems to be one corner case still left open, when we switch GPUs, but that still isn't fixed (and is not nearly as easy to reproduce as the bug originally discussed here).

Comment 83

[Cameron Dawson [:camd]]

Can we get someone with a macbook air to reproduce this and verify the fix?  My Macbook Pros both have the NVIDIA graphics, and doesn't exhibit the problem.

Comment 84

[Al Billings [:abillings - ex-MoCo]]

Using OS X 10.7.2 on my 11" inch air with http://mrdoob.github.com/three.js/examples/webgl_materials_cars_anaglyph.html, I see no difference between Firefox 9, beta 6 and Chrome. I just get the outlines of the car model in red and green.

In Firefox 8.0.1, I see the same behavior as Firefox 9b6.

With Firefox 7, I get a vehicle model on a background of a bridge. If this is the desired behavior, this is not fixed in 9b6.

My video card is Intel HD Graphics 3000 384 MB.

Comment 85

[Benoit Jacob [:bjacob] (mostly away)]

The  broken behavior you're seeing on 8+ is wanted. The bug was that contents of other windows would be shown in the webgl scene. The broken rendering is intentional, to avoid that security issue.

Comment 86

[u279076]

Benoit, so based on comment 84, can this bug be marked verified?

Comment 87

[Gregg Tavares]

(In reply to Al Billings [:abillings] from comment #84)
> Using OS X 10.7.2 on my 11" inch air with
> http://mrdoob.github.com/three.js/examples/webgl_materials_cars_anaglyph.
> html, I see no difference between Firefox 9, beta 6 and Chrome. I just get
> the outlines of the car model in red and green.

Just FYI, I submitted a patch to three.js to deal with this in three.js

https://github.com/mrdoob/three.js/pull/923

Comment 88

[Benoit Jacob [:bjacob] (mostly away)]

@ Anthony, yes it's verified.

Comment 89

[Benoit Jacob [:bjacob] (mostly away)]

@ Gregg, that's great. By the way, to help web devs test portability of their code, we now have a minimal-capability mode: webgl.min_capability_mode and a no-extensions mode: webgl.disable-extensions.

Comment 90

[Benoit Jacob [:bjacob] (mostly away)]

Can we unhide this bug now? It's fixed in Release and in ESR.

Comment 91

[Daniel Veditz [:dveditz]]

What's the status of the underlying Apple bug? Will we be exposing users of other Apple apps?

Comment 92

[Benoit Jacob [:bjacob] (mostly away)]

The underlying Apple bug isn't fixed. However, it's been discussed on public forum for a long time, as a Google search for | mac texture corruption | shows:

https://www.google.ca/search?q=mac+texture+corruption

It's even been blogged about publicly in the particular context of Firefox/WebGL, by the original bug reported here:

http://wahlers.com.br/claus/blog/talking-about-webgl-and-security/

Comment 93

[Benoit Jacob [:bjacob] (mostly away)]

To answer your other question precisely: yes, other applications are affected.

Comment 94

[Daniel Veditz [:dveditz]]

... but the issue is already public (comment 92) so OK to unhide it.