search

baldurk / renderdoc

RenderDoc is a stand-alone graphics debugging tool.
https://renderdoc.org
MIT License
8.6k stars 1.3k forks source link

Crash due to signed integer overflow in `GetByteSize` #3338

Closed Said6289 closed 1 month ago

Said6289 commented 1 month ago

Description

The function GetByteSize in renderdoc/driver/gl/gl_resources.cpp accepts three GLsizei arguments w, h, and d. The subexpression w * d * h is found in multiple places in the function and its result is undefined if the product overflows since these are 32-bit signed integers. I suspect the same problem exists in GetCompressedByteSize.

(gdb) bt
#0  OSUtility::ForceCrash () at renderdoc/os/posix/posix_specific.h:83
#1  0x000076a63d95213d in AllocAlignedBuffer (size=18446744071562067968, alignment=64) at renderdoc/common/common.cpp:214
#2  0x000076a63d17b7d6 in GLResourceManager::Serialise_InitialState<WriteSerialiser> (this=0x24bc4a50, ser=..., id=..., record=0x25def1b0, initial=0x28e9d858) at renderdoc/driver/gl/gl_initstate.cpp:1641
#3  0x000076a63d1724a7 in GLResourceManager::Serialise_InitialState (this=0x24bc4a50, ser=..., id=..., record=0x25def1b0, initial=0x28e9d858) at renderdoc/driver/gl/gl_initstate.cpp:1875
#4  0x000076a63d144a74 in ResourceManager<GLResourceManagerConfiguration>::InsertInitialContentsChunks (this=0x24bc4a50, ser=...) at renderdoc/core/resource_manager.h:1635
#5  0x000076a63d128cb2 in WrappedOpenGL::EndFrameCapture (this=0x76a63e645bb0 <glxhook+16>, devWnd=...) at renderdoc/driver/gl/gl_driver.cpp:2400
#6  0x000076a63d9747da in RenderDoc::EndFrameCapture (this=0x76a63e659760 <RenderDoc::Inst()::realInst>, devWnd=...) at renderdoc/core/core.cpp:881
#7  0x000076a63d127ac1 in WrappedOpenGL::SwapBuffers (this=0x76a63e645bb0 <glxhook+16>, winSystem=WindowingSystem::Xlib, windowHandle=0x1800009) at renderdoc/driver/gl/gl_driver.cpp:2189
#8  0x000076a63d534980 in glXSwapBuffers_renderdoc_hooked (dpy=0x24e6a6d0, drawable=25165833) at renderdoc/driver/gl/glx_hooks.cpp:536
#9  0x00000000009a05c3 in X11_GL_SwapWindow ()
(gdb) frame 2
#2  0x000076a63d17b7d6 in GLResourceManager::Serialise_InitialState<WriteSerialiser> (this=0x24bc4a50, ser=..., id=..., record=0x25def1b0, initial=0x28e9d858) at renderdoc/driver/gl/gl_initstate.cpp:1641
1641              byte *scratchBuf = AllocAlignedBuffer(size);
(gdb) p TextureState.width
$7 = 1024
(gdb) p TextureState.height
$8 = 1024
(gdb) p copySlices
$9 = 3072
(gdb) p fmt
$10 = eGL_RED
(gdb) p type
$11 = eGL_HALF_FLOAT
(gdb) p size
$12 = 18446744071562067968

Steps to reproduce

In my case I have single channel half float 3D texture that's 1024x1024x3072 which is 6 GiB in size and does not fit in a signed 32 bit number. Any capture that involves this resource causes the subprocess to crash due to the computed size being the result of undefined behavior as far as I can tell. Simply storing w, h, and d in size_t local variables results in a correct calculation and fixes the crash.

Environment

baldurk commented 1 month ago

I made fixes to GL and vulkan speculatively to handle this case, though on vulkan I wasn't able to successfully capture anything that needed >4GB and AFAIK allocations over 4GB are not supported on D3D at all so I didn't make any changes there. In theory this should work but it will likely depend heavily on OS/driver support at this point.