Closed
Bug 785734
(CVE-2012-5833)
Opened 12 years ago
Closed 12 years ago
Mesa crashes on certain texImage2D calls involving level>0
Categories
(Core :: Graphics: CanvasWebGL, defect)
Tracking
()
People
(Reporter: miaubiz, Assigned: bjacob)
References
Details
(Keywords: reporter-external, sec-critical, sec-vector, Whiteboard: [asan][adv-track-main17+][adv-track-esr17+][qa-] webgl-test-needed)
Attachments
(9 files, 3 obsolete files)
350 bytes,
text/plain
|
Details | |
902 bytes,
text/plain
|
Details | |
3.65 KB,
text/plain
|
Details | |
808 bytes,
text/plain
|
Details | |
327 bytes,
text/html
|
Details | |
864 bytes,
text/html
|
Details | |
3.96 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
3.84 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.29 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
<html>
<head>
<script>
var gl = document.createElement('canvas').getContext('experimental-webgl')
var texture = gl.createTexture()
gl.bindTexture(gl.TEXTURE_2D, texture)
gl.texImage2D(gl.TEXTURE_2D, 6, gl.RGBA, 512, 2, 0, gl.RGBA, gl.UNSIGNED_BYTE, null)
gl.deleteTexture(texture)
</script>
</head>
</html>
crashes on linux with mesa like:
==32234== ERROR: AddressSanitizer crashed on unknown address 0x000100000011 (pc 0x7fffc5321690 sp 0x7fffffff4988 bp 0x000000000006 T0)
AddressSanitizer can not provide additional info. ABORTING
#0 0x7fffc5321690 in ?? ??:0
also crashes chrome
Assignee | ||
Updated•12 years ago
|
OS: Mac OS X → Linux
Assignee | ||
Comment 4•12 years ago
|
||
I a debug build of Mesa, I get an assertion failure:
src/gallium/drivers/llvmpipe/lp_texture.c:395:llvmpipe_resource_map: Assertion `level == 0' failed.
Here, the value of level is 6 instead of the expected 0.
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007fffcc5bfb92 in _debug_assert_fail (expr=0x7fffccd9d3b0 "level == 0", file=0x7fffccd9d200 "src/gallium/drivers/llvmpipe/lp_texture.c", line=395, function=0x7fffccd9db30 "llvmpipe_resource_map")
at src/gallium/auxiliary/util/u_debug.c:278
278 os_abort();
(gdb) bt
#0 0x00007fffcc5bfb92 in _debug_assert_fail (expr=0x7fffccd9d3b0 "level == 0", file=0x7fffccd9d200 "src/gallium/drivers/llvmpipe/lp_texture.c", line=395, function=0x7fffccd9db30 "llvmpipe_resource_map")
at src/gallium/auxiliary/util/u_debug.c:278
#1 0x00007fffcc26a985 in llvmpipe_resource_map (resource=0x7fffd3bca000, level=6, layer=0, tex_usage=LP_TEX_USAGE_READ_WRITE, layout=LP_TEX_LAYOUT_LINEAR) at src/gallium/drivers/llvmpipe/lp_texture.c:395
#2 0x00007fffcc26b39d in llvmpipe_transfer_map (pipe=0x7fffde04c000, transfer=0x7fffde06bc00) at src/gallium/drivers/llvmpipe/lp_texture.c:699
#3 0x00007fffcc417088 in pipe_transfer_map (context=0x7fffde04c000, transfer=0x7fffde06bc00) at src/gallium/auxiliary/util/u_inlines.h:428
#4 0x00007fffcc417715 in st_texture_image_map (st=0x7fffd4e67000, stImage=0x7fffd44f3320, zoffset=0, usage=258, x=0, y=0, w=512, h=2) at src/mesa/state_tracker/st_texture.c:238
#5 0x00007fffcc40d76e in st_MapTextureImage (ctx=0x7fffd3a6d000, texImage=0x7fffd44f3320, slice=0, x=0, y=0, w=512, h=2, mode=6, mapOut=0x7fffffff8c08, rowStrideOut=0x7fffffff8c1c) at src/mesa/state_tracker/st_cb_texture.c:187
#6 0x00007fffcc3cafe3 in store_texsubimage (ctx=0x7fffd3a6d000, texImage=0x7fffd44f3320, xoffset=0, yoffset=0, zoffset=0, width=512, height=2, depth=1, format=6408, type=5121, pixels=0x7fffdda7d000, packing=0x7fffd3a73888,
caller=0x7fffccdec94a "glTexImage") at src/mesa/main/texstore.c:4352
#7 0x00007fffcc3cb206 in _mesa_store_teximage (ctx=0x7fffd3a6d000, dims=2, texImage=0x7fffd44f3320, format=6408, type=5121, pixels=0x7fffdda7d000, packing=0x7fffd3a73888) at src/mesa/main/texstore.c:4408
#8 0x00007fffcc40e13d in st_TexImage (ctx=0x7fffd3a6d000, dims=2, texImage=0x7fffd44f3320, format=6408, type=5121, pixels=0x7fffdda7d000, unpack=0x7fffd3a73888) at src/mesa/state_tracker/st_cb_texture.c:537
#9 0x00007fffcc3b6183 in teximage (ctx=0x7fffd3a6d000, dims=2, target=3553, level=6, internalFormat=6408, width=512, height=2, depth=1, border=0, format=6408, type=5121, pixels=0x7fffdda7d000) at src/mesa/main/teximage.c:2623
#10 0x00007fffcc3b6363 in _mesa_TexImage2D (target=3553, level=6, internalFormat=6408, width=512, height=2, border=0, format=6408, type=5121, pixels=0x7fffdda7d000) at src/mesa/main/teximage.c:2663
#11 0x00007ffff365a06f in mozilla::gl::GLContext::fTexImage2D (this=0x7fffd3a50000, target=3553, level=6, internalformat=6408, width=512, height=2, border=0, format=6408, type=5121, pixels=0x7fffdda7d000)
at ../../../dist/include/GLContext.h:2672
#12 0x00007ffff3672ced in mozilla::WebGLContext::CheckedTexImage2D (this=0x7fffce86d400, target=3553, level=6, internalFormat=6408, width=512, height=2, border=0, format=6408, type=5121, data=0x7fffdda7d000)
at /hack/mozilla-inbound/content/canvas/src/WebGLContextGL.cpp:5604
#13 0x00007ffff3673465 in mozilla::WebGLContext::TexImage2D_base (this=0x7fffce86d400, target=3553, level=6, internalformat=6408, width=512, height=2, srcStrideOrZero=0, border=0, format=6408, type=5121, data=0x0, byteLength=0,
jsArrayType=-1, srcFormat=mozilla::WebGLTexelConversions::Auto, srcPremultiplied=false) at /hack/mozilla-inbound/content/canvas/src/WebGLContextGL.cpp:5745
#14 0x00007ffff36736d1 in mozilla::WebGLContext::TexImage2D (this=0x7fffce86d400, cx=0x7fffd4394400, target=3553, level=6, internalformat=6408, width=512, height=2, border=0, format=6408, type=5121, pixels=0x0, rv=...)
Assignee | ||
Comment 5•12 years ago
|
||
Replacing 6 by 0 in the testcases avoids the crash, so this is specifically about setting a mipmap level >0 on a newly created texture.
Summary: firefox crashes with webgl, mesa → Mesa crashes when setting mipmap level>0 on a newly created texture
Assignee | ||
Comment 6•12 years ago
|
||
Crash confirmed on Mesa/Intel driver on another machine, so it's not just LLVMpipe.
Assignee | ||
Comment 7•12 years ago
|
||
Obviously the fact that the Mesa debug build asserts means it can't be used to assess how security critical this is.
In a release build of mesa, in valgrind, I get an invalid write, followed by an invalid read, and then the crash:
==26671== Invalid write of size 8
==26671== at 0x2C92A723: llvmpipe_resource_create (lp_texture.c:165)
==26671== by 0x2CA8E71B: st_texture_create (st_texture.c:94)
==26671== by 0x2CA879B0: guess_and_alloc_texture (st_cb_texture.c:393)
==26671== by 0x2CA89220: st_AllocTextureImageBuffer (st_cb_texture.c:447)
==26671== by 0x2CA52461: _mesa_store_teximage (texstore.c:4403)
==26671== by 0x2CA3C07E: teximage (teximage.c:2623)
==26671== by 0x2CA3D2E7: _mesa_TexImage2D (teximage.c:2663)
==26671== by 0x72E306E: mozilla::gl::GLContext::fTexImage2D(unsigned int, int, int, int, int, int, unsigned int, unsigned int, void const*) (GLContext.h:2672)
==26671== by 0x72FBCEC: mozilla::WebGLContext::CheckedTexImage2D(unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, void const*) (WebGLContextGL.cpp:5604)
==26671== by 0x72FC464: mozilla::WebGLContext::TexImage2D_base(unsigned int, int, unsigned int, int, int, int, int, unsigned int, unsigned int, void*, unsigned int, int, mozilla::WebGLTexelConversions::WebGLTexelFormat, bool) (WebGLContextGL.cpp:5745)
==26671== by 0x72FC6D0: mozilla::WebGLContext::TexImage2D(JSContext*, unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, mozilla::dom::TypedArray_base<unsigned char, &(JS_GetObjectAsArrayBufferView(JSContext*, JSObject*, unsigned int*, unsigned char**))>*, mozilla::ErrorResult&) (WebGLContextGL.cpp:5789)
==26671== by 0x83B6645: mozilla::dom::WebGLRenderingContextBinding::texImage2D(JSContext*, JS::Handle<JSObject*>, mozilla::WebGLContext*, unsigned int, JS::Value*) (WebGLRenderingContextBinding.cpp:4307)
==26671== Address 0x1a93b7d8 is 0 bytes after a block of size 664 alloc'd
==26671== at 0x402B7FA: calloc (vg_replace_malloc.c:591)
==26671== by 0x2C92A352: llvmpipe_resource_create (lp_texture.c:244)
==26671== by 0x2CA8E71B: st_texture_create (st_texture.c:94)
==26671== by 0x2CA879B0: guess_and_alloc_texture (st_cb_texture.c:393)
==26671== by 0x2CA89220: st_AllocTextureImageBuffer (st_cb_texture.c:447)
==26671== by 0x2CA52461: _mesa_store_teximage (texstore.c:4403)
==26671== by 0x2CA3C07E: teximage (teximage.c:2623)
==26671== by 0x2CA3D2E7: _mesa_TexImage2D (teximage.c:2663)
==26671== by 0x72E306E: mozilla::gl::GLContext::fTexImage2D(unsigned int, int, int, int, int, int, unsigned int, unsigned int, void const*) (GLContext.h:2672)
==26671== by 0x72FBCEC: mozilla::WebGLContext::CheckedTexImage2D(unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, void const*) (WebGLContextGL.cpp:5604)
==26671== by 0x72FC464: mozilla::WebGLContext::TexImage2D_base(unsigned int, int, unsigned int, int, int, int, int, unsigned int, unsigned int, void*, unsigned int, int, mozilla::WebGLTexelConversions::WebGLTexelFormat, bool) (WebGLContextGL.cpp:5745)
==26671== by 0x72FC6D0: mozilla::WebGLContext::TexImage2D(JSContext*, unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, mozilla::dom::TypedArray_base<unsigned char, &(JS_GetObjectAsArrayBufferView(JSContext*, JSObject*, unsigned int*, unsigned char**))>*, mozilla::ErrorResult&) (WebGLContextGL.cpp:5789)
==26671==
==26671== Invalid read of size 8
==26671== at 0x2C9940D0: xlib_displaytarget_map (xlib_sw_winsys.c:231)
==26671== by 0x2C92B0F4: llvmpipe_transfer_map (lp_texture.c:399)
==26671== by 0x2CA8E961: st_texture_image_map (u_inlines.h:428)
==26671== by 0x2CA875CC: st_MapTextureImage (st_cb_texture.c:187)
==26671== by 0x2CA472DE: store_texsubimage (texstore.c:4352)
==26671== by 0x2CA524B7: _mesa_store_teximage (texstore.c:4408)
==26671== by 0x2CA3C07E: teximage (teximage.c:2623)
==26671== by 0x2CA3D2E7: _mesa_TexImage2D (teximage.c:2663)
==26671== by 0x72E306E: mozilla::gl::GLContext::fTexImage2D(unsigned int, int, int, int, int, int, unsigned int, unsigned int, void const*) (GLContext.h:2672)
==26671== by 0x72FBCEC: mozilla::WebGLContext::CheckedTexImage2D(unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, void const*) (WebGLContextGL.cpp:5604)
==26671== by 0x72FC464: mozilla::WebGLContext::TexImage2D_base(unsigned int, int, unsigned int, int, int, int, int, unsigned int, unsigned int, void*, unsigned int, int, mozilla::WebGLTexelConversions::WebGLTexelFormat, bool) (WebGLContextGL.cpp:5745)
==26671== by 0x72FC6D0: mozilla::WebGLContext::TexImage2D(JSContext*, unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, mozilla::dom::TypedArray_base<unsigned char, &(JS_GetObjectAsArrayBufferView(JSContext*, JSObject*, unsigned int*, unsigned char**))>*, mozilla::ErrorResult&) (WebGLContextGL.cpp:5789)
==26671== Address 0x100000011 is not stack'd, malloc'd or (recently) free'd
==26671==
Assignee | ||
Comment 8•12 years ago
|
||
If I comment out the assert that we hit in comment 4, I get a crash in the debug build at the same location as the invalid read crash in comment 7, with some information:
Program received signal SIGSEGV, Segmentation fault.
0x00007fffcf216b14 in xlib_displaytarget_map (ws=0x7fffd3eb0ab0, dt=0x100000001, flags=3) at src/gallium/winsys/sw/xlib/xlib_sw_winsys.c:231
231 xlib_dt->mapped = xlib_dt->data;
(gdb) bt
#0 0x00007fffcf216b14 in xlib_displaytarget_map (ws=0x7fffd3eb0ab0, dt=0x100000001, flags=3) at src/gallium/winsys/sw/xlib/xlib_sw_winsys.c:231
#1 0x00007fffcf1899a8 in llvmpipe_resource_map (resource=0x7fffddf37800, level=6, layer=0, tex_usage=LP_TEX_USAGE_READ_WRITE, layout=LP_TEX_LAYOUT_LINEAR) at src/gallium/drivers/llvmpipe/lp_texture.c:399
#2 0x00007fffcf18a378 in llvmpipe_transfer_map (pipe=0x7fffd4913000, transfer=0x7fffd48e1740) at src/gallium/drivers/llvmpipe/lp_texture.c:699
#3 0x00007fffcf336068 in pipe_transfer_map (context=0x7fffd4913000, transfer=0x7fffd48e1740) at src/gallium/auxiliary/util/u_inlines.h:428
#4 0x00007fffcf3366f5 in st_texture_image_map (st=0x7fffd42f6000, stImage=0x7fffd3ebe6a0, zoffset=0, usage=258, x=0, y=0, w=512, h=2) at src/mesa/state_tracker/st_texture.c:238
#5 0x00007fffcf32c74e in st_MapTextureImage (ctx=0x7fffd2ad6000, texImage=0x7fffd3ebe6a0, slice=0, x=0, y=0, w=512, h=2, mode=6, mapOut=0x7fffffff8c08, rowStrideOut=0x7fffffff8c1c) at src/mesa/state_tracker/st_cb_texture.c:187
#6 0x00007fffcf2e9fc3 in store_texsubimage (ctx=0x7fffd2ad6000, texImage=0x7fffd3ebe6a0, xoffset=0, yoffset=0, zoffset=0, width=512, height=2, depth=1, format=6408, type=5121, pixels=0x7fffd51fa000, packing=0x7fffd2adc888,
caller=0x7fffcfd0b92a "glTexImage") at src/mesa/main/texstore.c:4352
#7 0x00007fffcf2ea1e6 in _mesa_store_teximage (ctx=0x7fffd2ad6000, dims=2, texImage=0x7fffd3ebe6a0, format=6408, type=5121, pixels=0x7fffd51fa000, packing=0x7fffd2adc888) at src/mesa/main/texstore.c:4408
#8 0x00007fffcf32d11d in st_TexImage (ctx=0x7fffd2ad6000, dims=2, texImage=0x7fffd3ebe6a0, format=6408, type=5121, pixels=0x7fffd51fa000, unpack=0x7fffd2adc888) at src/mesa/state_tracker/st_cb_texture.c:537
#9 0x00007fffcf2d5163 in teximage (ctx=0x7fffd2ad6000, dims=2, target=3553, level=6, internalFormat=6408, width=512, height=2, depth=1, border=0, format=6408, type=5121, pixels=0x7fffd51fa000) at src/mesa/main/teximage.c:2623
#10 0x00007fffcf2d5343 in _mesa_TexImage2D (target=3553, level=6, internalFormat=6408, width=512, height=2, border=0, format=6408, type=5121, pixels=0x7fffd51fa000) at src/mesa/main/teximage.c:2663
#11 0x00007ffff365a06f in mozilla::gl::GLContext::fTexImage2D (this=0x7fffde5f9000, target=3553, level=6, internalformat=6408, width=512, height=2, border=0, format=6408, type=5121, pixels=0x7fffd51fa000)
at ../../../dist/include/GLContext.h:2672
(gdb) p xlib_dt
$1 = (struct xlib_displaytarget *) 0x100000001
If this 0x100000001 is really fixed and can't be manipulated, then we're safe. Is it?
Reporter | ||
Comment 11•12 years ago
|
||
Reporter | ||
Comment 12•12 years ago
|
||
on intel with attachment 655907 [details] I can get atleast:
==14260== ERROR: AddressSanitizer crashed on unknown address 0x00005c008000 (pc 0x7fffb70fc1c4 sp 0x7fffffff48e0 bp 0x000000000007 T0)
==15327== ERROR: AddressSanitizer crashed on unknown address 0x000000000008 (pc 0x7fffb78d296b sp 0x7fffffff4810 bp 0x7fffcba1d880 T0)
==10880== ERROR: AddressSanitizer crashed on unknown address 0x000016004000 (pc 0x7fffb78fc48f sp 0x7fffffff48e0 bp 0x000000000000 T0)
==7359== ERROR: AddressSanitizer crashed on unknown address 0x000017002010 (pc 0x7ffff6f37e64 sp 0x7fffffff45e8 bp 0x7fffffff4840 T0)
==8053== ERROR: AddressSanitizer crashed on unknown address 0x00002e004010 (pc 0x7ffff6f37e64 sp 0x7fffffff45d8 bp 0x7fffffff4830 T0)
==11526== ERROR: AddressSanitizer crashed on unknown address 0x000000000000 (pc 0x7fffb74a9c39 sp 0x7fffffff4848 bp 0x7ffff6b4c080 T0)
==12790== ERROR: AddressSanitizer crashed on unknown address 0x00005e004010 (pc 0x7ffff6f37e64 sp 0x7fffffff45e8 bp 0x7fffffff4840 T0)
==13532== ERROR: AddressSanitizer crashed on unknown address 0x000078010000 (pc 0x7fffb78fc17a sp 0x7fffffff48e0 bp 0x000000000003 T0)
Assignee | ||
Comment 13•12 years ago
|
||
Hah. So this bug looks security-sensitive depending on the driver: sec-sensitive on the Intel driver at least (comment 12) but apparently not on LLVMpipe driver (comment 8).
Assignee | ||
Comment 14•12 years ago
|
||
This fixes it at least for the present testcase.
Attachment #656157 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #656157 -
Flags: review? → review?(jgilbert)
Assignee | ||
Comment 15•12 years ago
|
||
It's worth checking if a similar bug exists in glCopyTexImage2D.
Comment 16•12 years ago
|
||
Comment on attachment 656157 [details] [diff] [review]
work around the mesa teximage2d crasher by rejecting certain legal, but rare, teximage2d calls
Review of attachment 656157 [details] [diff] [review]:
-----------------------------------------------------------------
Gross, but necessary. Does this show up as a failure when the conformance suite is run? If not, we should add a test to make sure it does. We don't want to forget about something subtle (and hopefully temporary!) like this.
Attachment #656157 -
Flags: review?(jgilbert) → review+
Comment 17•12 years ago
|
||
We should actually be able to work around this in most cases, but it's probably not worth the effort when it should instead be fixed in Mesa.
Comment 18•12 years ago
|
||
Marking sec-critical during sec triage based on comment 12 addresses.
Keywords: sec-critical
Assignee | ||
Comment 19•12 years ago
|
||
You are right jgilbert, adding webgl-test-needed. I haven't actually checked if this is caught by the trunk test suite but let's default to assuming it isn't.
I am not convinced though that it really matters. If it does, rather than this fix, we should instead insert dummy texImage2D calls to allocate the level 0 images.
Assignee | ||
Updated•12 years ago
|
Whiteboard: webgl-test-needed
Comment 20•12 years ago
|
||
(In reply to Benoit Jacob [:bjacob] from comment #19)
> You are right jgilbert, adding webgl-test-needed. I haven't actually checked
> if this is caught by the trunk test suite but let's default to assuming it
> isn't.
>
> I am not convinced though that it really matters. If it does, rather than
> this fix, we should instead insert dummy texImage2D calls to allocate the
> level 0 images.
We are lucky that the workaround is simply uploading a zero'd texture. Since GLES requires level-0 to be specified for the texture to be complete, we can just emulate it being incomplete with zero'd level-0 data.
Updated•12 years ago
|
Keywords: sec-vector
Updated•12 years ago
|
status-firefox-esr10:
--- → ?
status-firefox17:
--- → affected
status-firefox18:
--- → affected
tracking-firefox18:
--- → +
Whiteboard: webgl-test-needed → [asan] webgl-test-needed
Updated•12 years ago
|
Assignee: nobody → bjacob
Assignee | ||
Comment 21•12 years ago
|
||
(In reply to Benoit Jacob [:bjacob] from comment #15)
> It's worth checking if a similar bug exists in glCopyTexImage2D.
Checked: there is a similar bug with glCopyTexImage2D. Attaching corresponding testcase.
Assignee | ||
Comment 22•12 years ago
|
||
Comment 23•12 years ago
|
||
Since it's a mesa crash we're working around and these are pretty basic functions I assume it affects all old branches. If so please remove the regressionwindow-wanted flag and set the appropriate branch status flags to "affected".
Keywords: regressionwindow-wanted
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 24•12 years ago
|
||
... and I have checked compressedTexImage2D too (with a DXT1 texture): it does NOT trigger the crash.
Going to do the above-discussed workaround which should be cheap enough (we should be able to get away with a 1x1 dummy texture) so we'll be able to just keep the workaround for all Mesa versions, rather than having to track Mesa versions.
Assignee | ||
Updated•12 years ago
|
Summary: Mesa crashes when setting mipmap level>0 on a newly created texture → Mesa crashes on certain texImage2D calls involving level>0
Assignee | ||
Comment 25•12 years ago
|
||
This new testcase nullifies the above patch -- it crashes even with this patch, in fact, it falls outside of the scope of what that patch works around.
APItrace:
453 glGenTextures(n = 1, textures = &2)
455 glBindTexture(target = GL_TEXTURE_2D, texture = 2)
458 glTexImage2D(target = GL_TEXTURE_2D, level = 0, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096))
462 glTexImage2D(target = GL_TEXTURE_2D, level = 1, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096))
466 glTexImage2D(target = GL_TEXTURE_2D, level = 2, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096))
470 glTexImage2D(target = GL_TEXTURE_2D, level = 3, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096))
474 glTexImage2D(target = GL_TEXTURE_2D, level = 4, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096))
478 glTexImage2D(target = GL_TEXTURE_2D, level = 5, internalformat = GL_RGBA, width = 512, height = 2, border = 0, format = GL_RGBA, type = GL_UNSIGNED_BYTE, pixels = blob(4096)) // incomplete
Attachment #656157 -
Attachment is obsolete: true
Assignee | ||
Comment 26•12 years ago
|
||
The crash condition appears to be:
level>0 AND width>=512 AND height>=2 AND (mipmap level 0 undefined OR mipmap level 0 has the same width)
Assignee | ||
Comment 27•12 years ago
|
||
This prevents crashes in all the variants of the testcase that I've been able to make.
Attachment #661222 -
Flags: review?(jgilbert)
Comment 28•12 years ago
|
||
Comment on attachment 661222 [details] [diff] [review]
work around the mesa teximage2d crasher by rejecting certain legal, but rare, teximage2d and copyTexImage2d calls
Review of attachment 661222 [details] [diff] [review]:
-----------------------------------------------------------------
I could be misreading this, but it looks like the conditions we test are wrong.
::: content/canvas/src/WebGLContextGL.cpp
@@ +908,5 @@
> + level > 0 &&
> + !sub)
> + {
> + size_t face = WebGLTexture::FaceForTarget(target);
> + WebGLTexture *tex = activeBoundTextureForTarget(target);
Is |tex| always going to be non-null?
@@ +910,5 @@
> + {
> + size_t face = WebGLTexture::FaceForTarget(target);
> + WebGLTexture *tex = activeBoundTextureForTarget(target);
> + if (!tex->HasImageInfoAt(0, face) ||
> + tex->ImageInfoAt(0, face).Width() >= width)
This doesn't seem in line with the crash condition you listed: "level>0 AND width>=512 AND height>=2 AND (mipmap level 0 undefined OR mipmap level 0 has the same width)"
Also, |tex->ImageInfoAt(0, face).Width() >= width| seems like it would be very common, since level >0 should always have a smaller width than level 0.
@@ +913,5 @@
> + if (!tex->HasImageInfoAt(0, face) ||
> + tex->ImageInfoAt(0, face).Width() >= width)
> + {
> + return ErrorInvalidOperation("%s: rejecting valid call to avoid Mesa driver crash. "
> + "Please go complain on https://bugzilla.mozilla.org/show_bug.cgi?id=785734",
Why are we telling people to complain on this bug, especially since this bug is currently a sec-bug, and thus invisible to most? Should we not tell them to complain to Mesa?
Attachment #661222 -
Flags: review?(jgilbert) → review-
Updated•12 years ago
|
Assignee | ||
Comment 29•12 years ago
|
||
Indeed: tex needed checking for non-null, and the inequality on width was backwards. I want to keep a reference to this bug so we get a chance to learn if the solution adopted here is harming real-world content (once this bug is un-hidden), but I simplified it.
Attachment #661222 -
Attachment is obsolete: true
Attachment #663519 -
Flags: review?(jgilbert)
Comment 30•12 years ago
|
||
Comment on attachment 663519 [details] [diff] [review]
work around the mesa teximage2d crasher by rejecting certain legal, but rare, teximage2d and copyTexImage2d calls
Review of attachment 663519 [details] [diff] [review]:
-----------------------------------------------------------------
One formatting nit.
::: content/canvas/src/WebGLContextGL.cpp
@@ +678,5 @@
> + {
> + if (!tex->HasImageInfoAt(0, face) ||
> + tex->ImageInfoAt(0, face).Width() <= width)
> + {
> + return ErrorInvalidOperation("%s: rejecting valid call to avoid Mesa driver crash. "
You have an extra space between 'return' and 'ErrorInvalidOperation'.
Attachment #663519 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 31•12 years ago
|
||
Target Milestone: --- → mozilla18
Comment 32•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 33•12 years ago
|
||
Comment on attachment 663519 [details] [diff] [review]
work around the mesa teximage2d crasher by rejecting certain legal, but rare, teximage2d and copyTexImage2d calls
Sorry for the delayed request.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Driver bug
User impact if declined: sec-critical
Testing completed (on m-c, etc.): been on m-c since last week
Risk to taking this patch (and alternatives if risky): very low risk, worst case we'd break some WebGL page, but this has been in m-c for half a week without a report of regression
String or UUID changes made by this patch: none
Attachment #663519 -
Flags: approval-mozilla-beta?
Attachment #663519 -
Flags: approval-mozilla-aurora?
Comment 34•12 years ago
|
||
Comment on attachment 663519 [details] [diff] [review]
work around the mesa teximage2d crasher by rejecting certain legal, but rare, teximage2d and copyTexImage2d calls
This missed our final beta, and doesn't seem critical for release. Approving for Aurora 17, however.
Attachment #663519 -
Flags: approval-mozilla-beta?
Attachment #663519 -
Flags: approval-mozilla-beta-
Attachment #663519 -
Flags: approval-mozilla-aurora?
Attachment #663519 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 35•12 years ago
|
||
Comment 36•12 years ago
|
||
Hey guys, are you sure a simple rejection of
GLint maxSize = MaxSizeForTarget(target) >> level;
if (width > maxSize || height > maxSize) {
// generate GL error INVALID_VALUE
doesn't handle the issue as well? I tested this on Chrome and it seemed to stop the asan issues though I might have missed one.
Assignee | ||
Comment 37•12 years ago
|
||
Hm, so here I am testing with a debug build of Mesa and I'm testing whether the Mesa assertion fails rather than whether a memory corruption happens (because I am lazy).
The condition, here, is:
GLint maxSize = 16384 >> level;
if (width > maxSize || height > maxSize) {
// generate GL error INVALID_VALUE
But MAX_TEXTURE_SIZE, as returned by Mesa, is 4096.
In any case, it does seem that the problem is symmetric between height and width, so I need to amend my patch in some way.
Assignee | ||
Comment 38•12 years ago
|
||
Reopening because the same issue occurs with height as it does with width -- not sure why I thought otherwise.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 39•12 years ago
|
||
More testing confirms comment 37 on my system; since your proposed restriction is stricter, it would indeed avoid the crash.
Assignee | ||
Comment 40•12 years ago
|
||
Attachment #668711 -
Flags: review?(jgilbert)
Comment 41•12 years ago
|
||
I added a conformance test under the assumption that Daniel's interpretation of the ES spec is correct
https://www.khronos.org/registry/webgl/sdk/tests/conformance/textures/texture-size-limit.html
Assignee | ||
Comment 42•12 years ago
|
||
Thanks!
You're right, indeed, in the GLES 2.0.25 spec page 69 it says that we should return INVALID_VALUE in this case.
Then I'll update the patch to 1) generate INVALID_VALUE and 2) do this check unconditionally, not just as a driver work-around.
Assignee | ||
Updated•12 years ago
|
Attachment #668711 -
Flags: review?(jgilbert)
Assignee | ||
Comment 43•12 years ago
|
||
Attachment #668711 -
Attachment is obsolete: true
Attachment #668732 -
Flags: review?(jgilbert)
Comment 44•12 years ago
|
||
Comment on attachment 668732 [details] [diff] [review]
correct patch (Gregg's idea)
Review of attachment 668732 [details] [diff] [review]:
-----------------------------------------------------------------
Interesting.
Attachment #668732 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 45•12 years ago
|
||
Assignee | ||
Comment 46•12 years ago
|
||
Comment on attachment 668732 [details] [diff] [review]
correct patch (Gregg's idea)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): driver bug
User impact if declined: sec-critical
Testing completed (on m-c, etc.): m-i
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: none
Attachment #668732 -
Flags: approval-mozilla-beta?
Attachment #668732 -
Flags: approval-mozilla-aurora?
Comment 47•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/81675e760c12
(I reset the status flags based on what I can tell happened here. Please change them back if I goofed)
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
status-firefox19:
--- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: mozilla18 → mozilla19
Updated•12 years ago
|
tracking-firefox17:
--- → +
Updated•12 years ago
|
Attachment #668732 -
Flags: approval-mozilla-beta?
Attachment #668732 -
Flags: approval-mozilla-beta+
Attachment #668732 -
Flags: approval-mozilla-aurora?
Attachment #668732 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 48•12 years ago
|
||
Comment 49•12 years ago
|
||
(In reply to Benoit Jacob [:bjacob] from comment #48)
> http://hg.mozilla.org/releases/mozilla-aurora/rev/fe15e7126620
> http://hg.mozilla.org/releases/mozilla-beta/rev/6feb23ee423c
Please prepare a patch for mozilla-esr10 as well. Thanks!
tracking-firefox-esr10:
--- → 17+
Assignee | ||
Comment 50•12 years ago
|
||
I threw into this patch the fix for the same issue in copyTexImage2D, bug 802778.
Attachment #675620 -
Flags: review?(jgilbert)
Updated•12 years ago
|
Attachment #675620 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 51•12 years ago
|
||
Comment on attachment 675620 [details] [diff] [review]
esr10 patch
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined:
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:
See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #675620 -
Flags: approval-mozilla-esr10?
Assignee | ||
Comment 52•12 years ago
|
||
Erm, again:
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec:critical
User impact if declined: sec:critical
Fix Landed on Version: 17+
Risk to taking this patch (and alternatives if risky): very low risk (simple, well tested already)
String or UUID changes made by this patch: none
Updated•12 years ago
|
Whiteboard: [asan] webgl-test-needed → [asan][adv-track-main17+] webgl-test-needed
Updated•12 years ago
|
Attachment #675620 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Comment 53•12 years ago
|
||
Updated•12 years ago
|
Whiteboard: [asan][adv-track-main17+] webgl-test-needed → [asan][adv-track-main17+][adv-track-esr17+] webgl-test-needed
Updated•12 years ago
|
Alias: CVE-2012-5833
Comment 54•12 years ago
|
||
Can miaubiz@gmail.com take a look and verify fixed on these config(s)? I currently don't have access to these particular configs in order to confirm. Thank you.
Comment 55•12 years ago
|
||
Swapping out verifyme for [qa-] given comment 54.
Keywords: verifyme
Whiteboard: [asan][adv-track-main17+][adv-track-esr17+] webgl-test-needed → [asan][adv-track-main17+][adv-track-esr17+][qa-] webgl-test-needed
Reporter | ||
Comment 56•12 years ago
|
||
Matt Wobensmith: how do I verify? it's fixed for me atleast :|
Comment 57•12 years ago
|
||
(In reply to miaubiz from comment #56)
> Matt Wobensmith: how do I verify? it's fixed for me atleast :|
If you cannot reproduce this crash anymore then I think it's safe to call this verified. Can you please confirm the following builds don't crash:
* Firefox 17.0.2
* Firefox 18.0.1
* Firefox 19.0b2
* Firefox 17.0.2esr
* Firefox 10.0.12esr
(listed in order of priority)
Thank you
Reporter | ||
Comment 58•12 years ago
|
||
couldn't find a firefox 17.0.2 non-esr
18.0 and 19.0b2 both say "Error: WebGL: texImage2D: the maximum texture size for level 6 is 128", as expected.
cheers
Comment 59•12 years ago
|
||
Sorry, the remaining builds are here:
* 17.0.1: ftp://ftp.mozilla.org/pub/firefox/releases/17.0.1/
* 17.0.2esr: ftp://ftp.mozilla.org/pub/firefox/releases/17.0.2esr/
* 10.0.12esr: ftp://ftp.mozilla.org/pub/firefox/releases/10.0.12esr/
Thank you
Reporter | ||
Comment 60•12 years ago
|
||
17.0.1 and 17.0.2esr give the same correct error message
10.0.12esr can't get a webgl context, presumably because webgl was blacklisted in 10.0.12esr, it is therefore not affected by the bug.
Assignee | ||
Comment 61•12 years ago
|
||
miaubiz, your time spent verifying this is very much appreciated!
Mesa is blacklisted in ESR10 as you noticed in comment 60.
Comment 62•12 years ago
|
||
Thank you very much, miaubuz. Marking this verified across all branches.
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
Group: core-security
Updated•6 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•