[PATCH] wined3d: Pass d3d_info instead of the device to wined3d_stateblock_state_init.
This fixes a NULL pointer dereference in device_init found by Coverity. Signed-off-by: Sven Baars <sven.wine(a)gmail.com> --- dlls/wined3d/device.c | 4 ++-- dlls/wined3d/stateblock.c | 6 +++--- dlls/wined3d/wined3d_private.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index d9d6cf2676..d1ab739c77 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -5093,7 +5093,7 @@ HRESULT CDECL wined3d_device_reset(struct wined3d_device *device, memset(&device->state, 0, sizeof(device->state)); state_init(&device->state, &device->fb, &device->adapter->d3d_info, WINED3D_STATE_INIT_DEFAULT); memset(&device->stateblock_state, 0, sizeof(device->stateblock_state)); - wined3d_stateblock_state_init(&device->stateblock_state, device, WINED3D_STATE_INIT_DEFAULT); + wined3d_stateblock_state_init(&device->stateblock_state, &device->adapter->d3d_info, WINED3D_STATE_INIT_DEFAULT); device->update_stateblock_state = &device->stateblock_state; device_init_swapchain_state(device, swapchain); @@ -5368,7 +5368,7 @@ HRESULT device_init(struct wined3d_device *device, struct wined3d *wined3d, } state_init(&device->state, &device->fb, &adapter->d3d_info, WINED3D_STATE_INIT_DEFAULT); - wined3d_stateblock_state_init(&device->stateblock_state, device, WINED3D_STATE_INIT_DEFAULT); + wined3d_stateblock_state_init(&device->stateblock_state, &adapter->d3d_info, WINED3D_STATE_INIT_DEFAULT); device->update_stateblock_state = &device->stateblock_state; device->max_frame_latency = 3; diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c index cd72209391..e66f6ab036 100644 --- a/dlls/wined3d/stateblock.c +++ b/dlls/wined3d/stateblock.c @@ -1536,7 +1536,7 @@ static void stateblock_state_init_default(struct wined3d_stateblock_state *state } void wined3d_stateblock_state_init(struct wined3d_stateblock_state *state, - const struct wined3d_device *device, DWORD flags) + const struct wined3d_d3d_info *d3d_info, DWORD flags) { unsigned int i; @@ -1546,7 +1546,7 @@ void wined3d_stateblock_state_init(struct wined3d_stateblock_state *state, } if (flags & WINED3D_STATE_INIT_DEFAULT) - stateblock_state_init_default(state, &device->adapter->d3d_info); + stateblock_state_init_default(state, d3d_info); } static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, @@ -1556,7 +1556,7 @@ static HRESULT stateblock_init(struct wined3d_stateblock *stateblock, stateblock->ref = 1; stateblock->device = device; - wined3d_stateblock_state_init(&stateblock->stateblock_state, device, 0); + wined3d_stateblock_state_init(&stateblock->stateblock_state, d3d_info, 0); if (type == WINED3D_SBT_RECORDED) return WINED3D_OK; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 57df9f1eb2..c722d6dab2 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -3632,7 +3632,7 @@ struct wined3d_stateblock void stateblock_init_contained_states(struct wined3d_stateblock *stateblock) DECLSPEC_HIDDEN; void wined3d_stateblock_state_init(struct wined3d_stateblock_state *state, - const struct wined3d_device *device, DWORD flags) DECLSPEC_HIDDEN; + const struct wined3d_d3d_info *d3d_info, DWORD flags) DECLSPEC_HIDDEN; void wined3d_stateblock_state_cleanup(struct wined3d_stateblock_state *state) DECLSPEC_HIDDEN; void wined3d_light_state_enable_light(struct wined3d_light_state *state, const struct wined3d_d3d_info *d3d_info, -- 2.17.1
On 20-02-19 14:05, Henri Verbeet wrote:
On Wed, 20 Feb 2019 at 16:12, Sven Baars <sven.wine(a)gmail.com> wrote:
This fixes a NULL pointer dereference in device_init found by Coverity.
I don't think so. Hi Henri,
In device_init is says device->adapter = wined3d->adapter_count ? adapter : NULL; meaning that device->adapter can be NULL if adapter_count can be 0. However, then I would actually expect the first line of device_init, which is struct wined3d_adapter *adapter = &wined3d->adapters[adapter_idx]; also to fail. I found that in wined3d_init it says that wined3d->adapter_count = 1; and I don't think this is updated anywhere, so then maybe one could assume it is always 1, but then why does it say device->adapter = wined3d->adapter_count ? adapter : NULL; in the first place? Or is this part of some cleanup that still has to be done? Best, Sven
On Wed, 20 Feb 2019 at 16:47, Sven Baars <sven.wine(a)gmail.com> wrote:
On 20-02-19 14:05, Henri Verbeet wrote: In device_init is says
device->adapter = wined3d->adapter_count ? adapter : NULL;
meaning that device->adapter can be NULL if adapter_count can be 0.
All callers validate that adapter_idx < adapter_count. The check in that line is redundant.
However, then I would actually expect the first line of device_init, which is
struct wined3d_adapter *adapter = &wined3d->adapters[adapter_idx];
also to fail.
Why would it?
assume it is always 1, but then why does it say
device->adapter = wined3d->adapter_count ? adapter : NULL;
in the first place? Or is this part of some cleanup that still has to be done?
History, mostly. At some point we didn't create adapters when using the GDI renderer. For reference, see commit 12788f6fc8987687453092cb3914fb46f2a4e4d1, which introduced the code in question.
On 20-02-19 14:56, Henri Verbeet wrote:
On Wed, 20 Feb 2019 at 16:47, Sven Baars <sven.wine(a)gmail.com> wrote:
On 20-02-19 14:05, Henri Verbeet wrote: In device_init is says
device->adapter = wined3d->adapter_count ? adapter : NULL;
meaning that device->adapter can be NULL if adapter_count can be 0. All callers validate that adapter_idx < adapter_count. The check in that line is redundant. Thanks for the explanation. Do you want me to replace "wined3d->adapter_count ? adapter : NULL" with "adapter"? I wouldn't mind if you did it either.
participants (2)
-
Henri Verbeet -
Sven Baars