On 01.12.2015 14:54, Bernhard Übelacker wrote:
https://bugs.winehq.org/show_bug.cgi?id=39667
Probably same issue as in https://bugs.winehq.org/show_bug.cgi?id=12432 . (Attached backtrace seems equal.)
Steps to reproduce:
- start launcher
- "Configure Controller"
- leave dialog with "Cancel"
- crash
MotoGP 3 demo launcher uses ConfigureDevices for the key mapping. The crash seems to happen because the result of a GetProperty(DIPROP_USERNAME) is used without checking.
This affects just the launcher. The game starts without problems with a default key mapping.
Signed-off-by: Bernhard Übelacker bernhardu@vr-web.de
dlls/dinput/device.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ dlls/dinput/device_private.h | 1 + dlls/dinput8/tests/device.c | 29 +++++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index e525f01..1095c4c 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -783,6 +783,7 @@ HRESULT _set_action_map(LPDIRECTINPUTDEVICE8W iface, LPDIACTIONFORMATW lpdiaf, L DIOBJECTDATAFORMAT *obj_df = NULL; DIPROPDWORD dp; DIPROPRANGE dpr;
- DIPROPSTRING dps; WCHAR username[MAX_PATH]; DWORD username_size = MAX_PATH; int i, action = 0, num_actions = 0;
@@ -863,6 +864,19 @@ HRESULT _set_action_map(LPDIRECTINPUTDEVICE8W iface, LPDIACTIONFORMATW lpdiaf, L else lstrcpynW(username, lpszUserName, MAX_PATH);
- dps.diph.dwSize = sizeof(dps);
- dps.diph.dwHeaderSize = sizeof(DIPROPHEADER);
- dps.diph.dwObj = 0;
- dps.diph.dwHow = DIPH_DEVICE;
- lstrcpynW(dps.wsz, username, sizeof(dps.wsz));
- IDirectInputDevice8_SetProperty(iface, DIPROP_USERNAME, &dps.diph);
- if (dwFlags & DIDSAM_NOUSER)
- {
HeapFree(GetProcessHeap(), 0, This->username);This->username = NULL;- }
When I understand it correctly, if DISAM_NOUSER is set, you set the username just to reset it manually again afterwards? Why not pass an empty string to IDirectInputDevice8_SetProperty?
- /* Save the settings to disk */ save_mapping_settings(iface, lpdiaf, username);
@@ -1100,6 +1114,9 @@ ULONG WINAPI IDirectInputDevice2WImpl_Release(LPDIRECTINPUTDEVICE8W iface) /* Free action mapping */ HeapFree(GetProcessHeap(), 0, This->action_map);
- /* Free username */
- HeapFree(GetProcessHeap(), 0, This->username);
- EnterCriticalSection( &This->dinput->crit ); list_remove( &This->entry ); LeaveCriticalSection( &This->dinput->crit );
@@ -1251,6 +1268,17 @@ HRESULT WINAPI IDirectInputDevice2WImpl_GetProperty(LPDIRECTINPUTDEVICE8W iface, TRACE("buffersize = %d\n", pd->dwData); break; }
case (DWORD_PTR) DIPROP_USERNAME:{LPDIPROPSTRING ps = (LPDIPROPSTRING)pdiph;if (pdiph->dwSize != sizeof(DIPROPSTRING)) return DIERR_INVALIDPARAM;ps->wsz[0] = 0;if (This->username)lstrcpynW(ps->wsz, This->username, sizeof(ps->wsz));
The last argument is wrong, it should be a WCHAR count.
break;} case (DWORD_PTR) DIPROP_VIDPID: FIXME("DIPROP_VIDPID not implemented\n"); return DIERR_UNSUPPORTED;@@ -1324,6 +1352,22 @@ HRESULT WINAPI IDirectInputDevice2WImpl_SetProperty( LeaveCriticalSection(&This->crit); break; }
case (DWORD_PTR) DIPROP_USERNAME:{LPCDIPROPSTRING ps = (LPCDIPROPSTRING)pdiph;if (pdiph->dwSize != sizeof(DIPROPSTRING)) return DIERR_INVALIDPARAM;if (!This->username)This->username = HeapAlloc(GetProcessHeap(), 0, sizeof(ps->wsz));if (!This->username)return DIERR_OUTOFMEMORY;This->username[0] = 0;if (ps->wsz)lstrcpynW(This->username, ps->wsz, sizeof(ps->wsz));
Same here. Also, it could make sense to look at the actual string length, instead of allocating a fixed size buffer.
break;} default: WARN("Unknown property %s\n", debugstr_guid(rguid)); return DIERR_UNSUPPORTED;diff --git a/dlls/dinput/device_private.h b/dlls/dinput/device_private.h index 52bbec4..26ec7c2 100644 --- a/dlls/dinput/device_private.h +++ b/dlls/dinput/device_private.h @@ -81,6 +81,7 @@ struct IDirectInputDeviceImpl /* Action mapping */ int num_actions; /* number of actions mapped */ ActionMap *action_map; /* array of mappings */
- WCHAR *username; /* set in SetActionMap */
};
extern BOOL get_app_key(HKEY*, HKEY*) DECLSPEC_HIDDEN; diff --git a/dlls/dinput8/tests/device.c b/dlls/dinput8/tests/device.c index 6495559..af8667d 100644 --- a/dlls/dinput8/tests/device.c +++ b/dlls/dinput8/tests/device.c @@ -223,8 +223,8 @@ static BOOL CALLBACK enumeration_callback(const DIDEVICEINSTANCEA *lpddi, IDirec dps.wsz[0] = '\0';
hr = IDirectInputDevice_GetProperty(lpdid, DIPROP_USERNAME, &dps.diph);
- todo_wine ok (SUCCEEDED(hr), "GetProperty failed hr=%08x\n", hr);
- todo_wine ok (!lstrcmpW(usernameW, dps.wsz), "Username not set correctly expected=%s, got=%s\n", wine_dbgstr_wn(usernameW, -1), wine_dbgstr_wn(dps.wsz, -1));
ok (SUCCEEDED(hr), "GetProperty failed hr=%08x\n", hr);
ok (!lstrcmpW(usernameW, dps.wsz), "Username not set correctly expected=%s, got=%s\n", wine_dbgstr_wn(usernameW, -1), wine_dbgstr_wn(dps.wsz, -1));
/* Test buffer size */ memset(&dp, 0, sizeof(dp));
@@ -275,6 +275,7 @@ static void test_action_mapping(void) HINSTANCE hinst = GetModuleHandleA(NULL); IDirectInput8A *pDI = NULL; DIACTIONFORMATA af;
- DIPROPSTRING dps; struct enum_data data = {pDI, &af, NULL, NULL, NULL, 0}; HWND hwnd;
@@ -342,6 +343,30 @@ static void test_action_mapping(void)
af.dwDataSize = 4 * sizeof(actionMapping) / sizeof(actionMapping[0]); af.dwNumActions = sizeof(actionMapping) / sizeof(actionMapping[0]);
/* test DIDSAM_NOUSER */dps.diph.dwSize = sizeof(dps);dps.diph.dwHeaderSize = sizeof(DIPROPHEADER);dps.diph.dwObj = 0;dps.diph.dwHow = DIPH_DEVICE;dps.wsz[0] = '\0';hr = IDirectInputDevice_GetProperty(data.keyboard, DIPROP_USERNAME, &dps.diph);ok (SUCCEEDED(hr), "GetProperty failed hr=%08x\n", hr);ok (dps.wsz[0] != 0, "Expected any username, got=%s\n", wine_dbgstr_wn(dps.wsz, -1));hr = IDirectInputDevice8_SetActionMap(data.keyboard, data.lpdiaf, NULL, DIDSAM_NOUSER);ok (SUCCEEDED(hr), "SetActionMap failed hr=%08x\n", hr);dps.diph.dwSize = sizeof(dps);dps.diph.dwHeaderSize = sizeof(DIPROPHEADER);dps.diph.dwObj = 0;dps.diph.dwHow = DIPH_DEVICE;dps.wsz[0] = '\0';hr = IDirectInputDevice_GetProperty(data.keyboard, DIPROP_USERNAME, &dps.diph);ok (SUCCEEDED(hr), "GetProperty failed hr=%08x\n", hr);ok (dps.wsz[0] == 0, "Expected empty username, got=%s\n", wine_dbgstr_wn(dps.wsz, -1));}
if (data.mouse != NULL)
-
Sebastian Lackner