1571914 - API violation in ReadbackManagerD3D11::ProcessTasks()

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect, P3)

Description

[mozillabugs]

ReadbackManagerD3D11::ProcessTasks() (gfx/layers/d3d11/ReadbackManagerD3D11.cpp) calls ID3D10Texture2D::Map(), then ID3D10Texture2D::Unmap() without first checking the return value from Map():

137:    D3D10_MAPPED_TEXTURE2D mappedTex;
138:    nextReadbackTask->mReadbackTexture->Map(0, D3D10_MAP_READ, 0, &mappedTex);
139:    nextReadbackTask->mReadbackTexture->Unmap(0);

https://docs.microsoft.com/en-us/windows/win32/api/d3d10/nf-d3d10-id3d10texture2d-unmap says "A subresource must be mapped before Unmap is called" but https://docs.microsoft.com/en-us/windows/win32/api/d3d10/nf-d3d10-id3d10texture2d-map indicates that Map() can fail, and refers to https://docs.microsoft.com/en-us/windows/win32/api/d3d10/nf-d3d10-id3d10texture1d-map , which says that one common failure is DXGI_ERROR_DEVICE_REMOVED. https://docs.microsoft.com/en-us/windows/uwp/gaming/handling-device-lost-scenarios says that this occurs, among other times, when

    o The graphics driver is upgraded.
    o The system changes from a power-saving graphics adapter to a performance graphics adapter.
    o The graphics device stops responding and is reset.
    o A graphics adapter is physically attached or removed.

and then goes on to say

Any time your app receives the DXGI_ERROR_DEVICE_REMOVED error, it must reinitialize the Direct3D device and recreate any device-dependent resources. Release any references to graphics device resources created with the previous Direct3D device; those resources are now invalid, and all references to the swap chain must be released before a new one can be created.

Comment 1

[Daniel Veditz [:dveditz]]

Bas: if this goes wrong what happens if we use the unmapped texture?

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to Daniel Veditz [:dveditz] from comment #1)

Bas: if this goes wrong what happens if we use the unmapped texture?

Well, we never use it, obviously, since as the comments say we're simply using the API as a sync point here, we don't really care if it succeeds or fails. Having said that, I believe Map sets a nullptr if it fails, so if Map failed, you'd get a null pointer dereference.

If map succeeded, and then unmap succeeds, and -then- you'd try to access the mappedText data pointer then that could lead to accessing unmapped memory. Obviously, we don't do that here, nor does it have anything to do with the initially reported bug of not handling the Map return value. I do not believe there to be an actual bug here.

Comment 3

[mozillabugs]

(In reply to Bas Schouten (:bas.schouten) from comment #2)

(In reply to Daniel Veditz [:dveditz] from comment #1)

Bas: if this goes wrong what happens if we use the unmapped texture?

Well, we never use it, obviously, since as the comments say we're simply using the API as a sync point here, we don't really care if it succeeds or fails. Having said that, I believe Map sets a nullptr if it fails, so if Map failed, you'd get a null pointer dereference.

The code doesn't use 'mappedTex'. It does, however, continue to use the ID3D10Texture2D that mReadbackTexture points to, despite MS saying that it's invalid to do so (see the last quote in comment 0). I'll try to produce a POC.

Comment 4

[Bas Schouten (:bas.schouten)]

(In reply to mozillabugs from comment #3)

(In reply to Bas Schouten (:bas.schouten) from comment #2)

(In reply to Daniel Veditz [:dveditz] from comment #1)

Bas: if this goes wrong what happens if we use the unmapped texture?

Well, we never use it, obviously, since as the comments say we're simply using the API as a sync point here, we don't really care if it succeeds or fails. Having said that, I believe Map sets a nullptr if it fails, so if Map failed, you'd get a null pointer dereference.

The code doesn't use 'mappedTex'. It does, however, continue to use the ID3D10Texture2D that mReadbackTexture points to, despite MS saying that it's invalid to do so (see the last quote in comment 0). I'll try to produce a POC.

That's fine, that happens in lots and lots of places. We continue to use all kinds of textures from a reset device for the duration of a frame after a device reset fails. All the calls on them will fail and the next frame we will reset the device.