[PATCH v2 0/4] MR358: atl: Avoid NULL pointer reference in AtlComModuleGetClassObject().
Fix crash in CAXA CAD when clicking component library.
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com
-- v2: atl: Avoid NULL pointer reference in AtlComModuleRevokeClassObjects(). atl: Avoid NULL pointer reference in AtlComModuleRegisterClassObjects(). atl: Avoid NULL pointer reference in AtlComModuleGetClassObject(). atl110: Add importlib.
From: Zhiyi Zhang zzhang@codeweavers.com
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/atl110/Makefile.in | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/atl110/Makefile.in b/dlls/atl110/Makefile.in index 49ba9338125..d33d30d2820 100644 --- a/dlls/atl110/Makefile.in +++ b/dlls/atl110/Makefile.in @@ -1,4 +1,5 @@ MODULE = atl110.dll +IMPORTLIB = atl110 IMPORTS = oleaut32 ole32 user32 gdi32 advapi32 uuid shlwapi EXTRADEFS = -D_ATL_VER=_ATL_VER_110 PARENTSRC = ../atl
From: Zhiyi Zhang zzhang@codeweavers.com
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- configure.ac | 1 + dlls/atl/atl.c | 4 +-- dlls/atl100/tests/atl.c | 27 +++++++++++++++ dlls/atl110/tests/Makefile.in | 6 ++++ dlls/atl110/tests/atl.c | 65 +++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 dlls/atl110/tests/Makefile.in create mode 100644 dlls/atl110/tests/atl.c
diff --git a/configure.ac b/configure.ac index d5791bab7b1..5ea0d9120f0 100644 --- a/configure.ac +++ b/configure.ac @@ -2394,6 +2394,7 @@ WINE_CONFIG_MAKEFILE(dlls/atl/tests) WINE_CONFIG_MAKEFILE(dlls/atl100) WINE_CONFIG_MAKEFILE(dlls/atl100/tests) WINE_CONFIG_MAKEFILE(dlls/atl110) +WINE_CONFIG_MAKEFILE(dlls/atl110/tests) WINE_CONFIG_MAKEFILE(dlls/atl80) WINE_CONFIG_MAKEFILE(dlls/atl80/tests) WINE_CONFIG_MAKEFILE(dlls/atl90) diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index 87a24e33ed7..d501e7a6d76 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -482,7 +482,7 @@ HRESULT WINAPI AtlComModuleGetClassObject(_ATL_COM_MODULE *pm, REFCLSID rclsid, return E_INVALIDARG;
for(iter = pm->m_ppAutoObjMapFirst; iter < pm->m_ppAutoObjMapLast; iter++) { - if(IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { + if(*iter && IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { if(!(*iter)->pCF) hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&(*iter)->pCF); if((*iter)->pCF) @@ -507,7 +507,7 @@ HRESULT WINAPI AtlComModuleGetClassObject(_ATL_COM_MODULE *pm, REFCLSID rclsid, return E_INVALIDARG;
for(iter = pm->m_ppAutoObjMapFirst; iter < pm->m_ppAutoObjMapLast; iter++) { - if(IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { + if(*iter && IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { if(!(*iter)->pCache->pCF) hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&(*iter)->pCache->pCF); if((*iter)->pCache->pCF) diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index e161878f8ea..e002af0d24c 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1062,6 +1062,32 @@ static void test_AtlAxCreateControl(void) DestroyWindow(hwnd); }
+static void test_AtlComModuleGetClassObject(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + void *ret; + + /* Test NULL module */ + hr = AtlComModuleGetClassObject(NULL, &GUID_NULL, &IID_NULL, &ret); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1077,6 +1103,7 @@ START_TEST(atl) test_ax_win(); test_AtlAxAttachControl(); test_AtlAxCreateControl(); + test_AtlComModuleGetClassObject();
CoUninitialize(); } diff --git a/dlls/atl110/tests/Makefile.in b/dlls/atl110/tests/Makefile.in new file mode 100644 index 00000000000..6374117e1be --- /dev/null +++ b/dlls/atl110/tests/Makefile.in @@ -0,0 +1,6 @@ +TESTDLL = atl110.dll +IMPORTS = uuid ole32 atl110 +EXTRADEFS = -D_ATL_VER=_ATL_VER_110 + +C_SRCS = \ + atl.c diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c new file mode 100644 index 00000000000..cb8f667596b --- /dev/null +++ b/dlls/atl110/tests/atl.c @@ -0,0 +1,65 @@ +/* + * Copyright 2022 Zhiyi Zhang for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> +#include <stdio.h> + +#define COBJMACROS +#define CONST_VTABLE + +#include <windef.h> +#include <winbase.h> +#include <winuser.h> +#include <atlbase.h> + +#include <wine/test.h> + +static void test_AtlComModuleGetClassObject(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + void *ret; + + /* Test NULL module */ + hr = AtlComModuleGetClassObject(NULL, &GUID_NULL, &IID_NULL, &ret); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); +} + +START_TEST(atl) +{ + CoInitialize(NULL); + + test_AtlComModuleGetClassObject(); + + CoUninitialize(); +}
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118419
Your paranoid android.
=== w10pro64_zh_CN (64 bit report) ===
atl100: atl.c:789: Test failed: DeleteFile failed! atl.c:1016: Test failed: failed to create file atl.c:1018: Test failed: WriteFile failed atl.c:1051: Test failed: DeleteFile failed!
From: Zhiyi Zhang zzhang@codeweavers.com
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/atl/atl.c | 4 ++-- dlls/atl100/tests/atl.c | 28 ++++++++++++++++++++++++++++ dlls/atl110/tests/atl.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index d501e7a6d76..5a302621d60 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -538,7 +538,7 @@ HRESULT WINAPI AtlComModuleRegisterClassObjects(_ATL_COM_MODULE *module, DWORD c return E_INVALIDARG;
for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { - if(!(*iter)->pfnGetClassObject) + if(!(*iter) || !(*iter)->pfnGetClassObject) continue;
hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&unk); @@ -566,7 +566,7 @@ HRESULT WINAPI AtlComModuleRegisterClassObjects(_ATL_COM_MODULE *module, DWORD c return E_INVALIDARG;
for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { - if(!(*iter)->pfnGetClassObject) + if(!(*iter) || !(*iter)->pfnGetClassObject) continue;
hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&unk); diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index e002af0d24c..7da6e5a9cd6 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1088,6 +1088,33 @@ static void test_AtlComModuleGetClassObject(void) ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); }
+static void test_AtlComModuleRegisterClassObjects(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRegisterClassObjects(NULL, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1104,6 +1131,7 @@ START_TEST(atl) test_AtlAxAttachControl(); test_AtlAxCreateControl(); test_AtlComModuleGetClassObject(); + test_AtlComModuleRegisterClassObjects();
CoUninitialize(); } diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c index cb8f667596b..9e4bad87e97 100644 --- a/dlls/atl110/tests/atl.c +++ b/dlls/atl110/tests/atl.c @@ -55,11 +55,39 @@ static void test_AtlComModuleGetClassObject(void) ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); }
+static void test_AtlComModuleRegisterClassObjects(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRegisterClassObjects(NULL, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { CoInitialize(NULL);
test_AtlComModuleGetClassObject(); + test_AtlComModuleRegisterClassObjects();
CoUninitialize(); }
From: Zhiyi Zhang zzhang@codeweavers.com
Signed-off-by: Zhiyi Zhang zzhang@codeweavers.com --- dlls/atl/atl.c | 6 ++++++ dlls/atl100/tests/atl.c | 26 ++++++++++++++++++++++++++ dlls/atl110/tests/atl.c | 26 ++++++++++++++++++++++++++ 3 files changed, 58 insertions(+)
diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index 5a302621d60..32e0c722f8b 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -598,6 +598,9 @@ HRESULT WINAPI AtlComModuleRevokeClassObjects(_ATL_COM_MODULE *module) return E_INVALIDARG;
for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { + if(!(*iter)) + continue; + hres = CoRevokeClassObject((*iter)->dwRegister); if(FAILED(hres)) return hres; @@ -617,6 +620,9 @@ HRESULT WINAPI AtlComModuleRevokeClassObjects(_ATL_COM_MODULE *module) return E_INVALIDARG;
for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { + if(!(*iter)) + continue; + hres = CoRevokeClassObject((*iter)->pCache->dwRegister); if(FAILED(hres)) return hres; diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index 7da6e5a9cd6..a40c38df25a 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1115,6 +1115,31 @@ static void test_AtlComModuleRegisterClassObjects(void) ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); }
+static void test_AtlComModuleRevokeClassObjects(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRevokeClassObjects(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1132,6 +1157,7 @@ START_TEST(atl) test_AtlAxCreateControl(); test_AtlComModuleGetClassObject(); test_AtlComModuleRegisterClassObjects(); + test_AtlComModuleRevokeClassObjects();
CoUninitialize(); } diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c index 9e4bad87e97..ba5639178c3 100644 --- a/dlls/atl110/tests/atl.c +++ b/dlls/atl110/tests/atl.c @@ -82,12 +82,38 @@ static void test_AtlComModuleRegisterClassObjects(void) ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); }
+static void test_AtlComModuleRevokeClassObjects(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRevokeClassObjects(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { CoInitialize(NULL);
test_AtlComModuleGetClassObject(); test_AtlComModuleRegisterClassObjects(); + test_AtlComModuleRevokeClassObjects();
CoUninitialize(); }
This merge request was approved by Jacek Caban.
-
Jacek Caban (＠jacek) -
Marvin -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)