Issue951851
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2004-05-11 11:02 by mhammond, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
dumpbin.txt | theller, 2004-05-11 15:45 | Output of 'dumpbin.exe /all \windows\system32\wmi.dll' | ||
wmi_crash.patch | mhammond, 2004-05-12 01:56 | New patch | ||
dynload_win.c-1.patch | theller, 2004-06-01 17:45 | dynload_win.c-1.patch | ||
dynload_win.c-2.patch | theller, 2004-06-01 17:46 | dynload_win.c-2.patch |
Messages (15) | |||
---|---|---|---|
msg20759 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-05-11 11:02 | |
As diagnosed by Thomas Heller, via the python-win32 list. On Windows 2000, if your sys.path includes the Windows system32 directory, 'import wmi' will crash Python. To reproduce, change to the system32 directory, start Python, and execute 'import wmi'. Note that Windows XP does not crash. The problem is in GetPythonImport(), in code that tries to walk the PE 'import table'. AFAIK, this is code that checks the correct Python version is used, but I've never seen this code before. I'm not sure why the code is actually crashing (ie, what assumptions made about the import table are wrong), but I have a patch that checks a the pointers are valid before they are de-referenced. After the patch is applied, the result is the correct: "ImportError: dynamic module does not define init function (initwmi)" exception. Assigning to thomas for his review, then I'm happy to check in. I propose this as a 2.3 bugfix. |
|||
msg20760 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-05-11 11:05 | |
Logged In: YES user_id=14198 Actually, I guess a comment regarding the pointer checks and referencing this bug would be nice :) |
|||
msg20761 - (view) | Author: Thomas Heller (theller) * | Date: 2004-05-11 14:49 | |
Logged In: YES user_id=11105 IMO, IsBadReadPointer(import_data, 12 + sizeof(DWORD)) should be enough. Yes, please add a comment in the code. This is a little bit hackish, but it fixes the problem. And the real problem can always be fixed later, if needed. And, BTW, python 2.3.3 crashes on Windows XP as well. |
|||
msg20762 - (view) | Author: Tim Peters (tim.peters) * | Date: 2004-05-11 15:20 | |
Logged In: YES user_id=31435 Mark, while you may not have seen this code before, you checked it in <wink -- rev 2.7>. IIRC, though, the person who *created* the patch was never heard from again. I don't understand what the code thinks it's doing either, exactly. The obvious concern: if the import table format has changed, won't we also fail to import legit C extensions? I haven't seen that happen yet, but I haven't yet built any extensions using VC 7.1 either. In any case, I'd agree it's better to get a mysterious import failure than a crash. Maybe the detail in the ImportError could be changed to indicate whan an import failure is due to a bad pointer. |
|||
msg20763 - (view) | Author: Thomas Heller (theller) * | Date: 2004-05-11 15:42 | |
Logged In: YES user_id=11105 Tim, I don't think the import table format has changed, instead wmi.dll doesn't have an import table (for whatever reason). Maybe the code isn't able to handle that correctly. Since Python 2.3 as well at it's extensions are still built with MSVC 6, I *think* we should be safe with this code. I'll attach the output of running MSVC.NET 2003's 'dumpbin.exe \windows\system32\wmi.dll' on my WinXP Pro SP1 for the curious. |
|||
msg20764 - (view) | Author: Thomas Heller (theller) * | Date: 2004-05-11 15:45 | |
Logged In: YES user_id=11105 Oh, we have to give the /all option to dumpbin ;-) |
|||
msg20765 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-05-11 23:00 | |
Logged In: YES user_id=14198 OK - will change to 12+so(WORD) And yes, I had seen this code - I meant "familiar with" :) Tim: Note that the import failure is not due to a bad import table (but the crash was). This code is trying to determine if a different version of Python is used. We are effectively skipping that check, and landing directly in the "does it have an init function?", then faling normally - ie, the code is now correctly *not* finding other Python versions linked against it. Thus, a different error message doesn't make much sense to me. |
|||
msg20766 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-05-12 01:56 | |
Logged In: YES user_id=14198 Seeing as it was the de-referencing of 'import_name' that crashed, I think a better patch is to terminate the outer while look as soon as we hit a bad string. Otherwise, this outer loop continues, 20 bytes at a time, until the outer pointer ('import_data') also goes bad or happens to reference \0. Attaching a slightly different patch, with comments and sizeof() change. |
|||
msg20767 - (view) | Author: Thomas Heller (theller) * | Date: 2004-06-01 17:45 | |
Logged In: YES user_id=11105 The reason the current code crashed when Python tries to import Win2k's or XP's wmi.dll as extension is that the size of the import table in this dll is zero. The first patch 'dynload_win.c-1.patch' fixes this by returning NULL in that case. The code, however, doesn't do what is intended in a debug build of Python. It looks for imports of 'python23.dll', when it should look for 'python23_d.dll' instead. The second patch 'dynload_win.c-2.patch' fixes this also (and includes the first patch as well). |
|||
msg20768 - (view) | Author: Thomas Heller (theller) * | Date: 2004-06-01 18:35 | |
Logged In: YES user_id=11105 This is not yet accepted. |
|||
msg20769 - (view) | Author: Thomas Heller (theller) * | Date: 2004-06-28 21:28 | |
Logged In: YES user_id=11105 It seems Mark doesn't listen (or don't have time). I'd like to check this in for 2.4. Any objections? |
|||
msg20770 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-06-28 23:20 | |
Logged In: YES user_id=14198 I'm sorry, but I'm not sure what I don't listen to :) You are correct about being short of time though. What would you like me to do? |
|||
msg20771 - (view) | Author: Thomas Heller (theller) * | Date: 2004-06-29 19:46 | |
Logged In: YES user_id=11105 Sorry if I was confusing. I simply want to know if this should be checked in or not. Maybe you can review the code, and/or try it out. Thanks. |
|||
msg20772 - (view) | Author: Mark Hammond (mhammond) * | Date: 2004-07-01 00:07 | |
Logged In: YES user_id=14198 Sorry about that - I thought my "IsBadReadPtr()" patch did end up getting checked in :( It is still sitting as a diff in my 2.3 tree, which is a shame. Sorry about that. You patch works fine with the 2.4 branch, and does also stop the crash, so please check it in. |
|||
msg20773 - (view) | Author: Thomas Heller (theller) * | Date: 2004-07-02 08:54 | |
Logged In: YES user_id=11105 Commited as dynload_win.c rev 2.12.14.1 and rev 2.13. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:04 | admin | set | github: 40243 |
2004-05-11 11:02:42 | mhammond | create |