classification
Title: winreg.SetValueEx causes crash if value = None
Type: crash Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: Claudiu.Popa, dmo2118, eryksun, jpe, python-dev, steve.dower, stutzbach, zach.ware
Priority: normal Keywords: patch

Created on 2014-04-04 03:17 by dmo2118, last changed 2014-07-03 16:08 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
fix-none-value.diff jpe, 2014-04-14 19:00 review
Messages (7)
msg215486 - (view) Author: Dave Odell (dmo2118) Date: 2014-04-04 03:17
Here's a small program that crashes Python 3.

import winreg
winreg.SetValueEx(winreg.HKEY_CURRENT_USER, 'Value', 0, 3, None)

I get a 0xC0000374 exception (STATUS_HEAP_CORRUPTION) when trying to run this. Here's a stack dump:

(snip)
ntdll.dll!RtlpLogHeapFailure+0xa4
ntdll.dll! ?? ::FNODOBFM::`string'+0x10c7c
kernel32.dll!HeapFree+0xa
MSVCR100.dll!free+0x1c
python34.dll!PySetValueEx+0xf8
python34.dll!PyCFunction_Call+0x12d
python34.dll!call_function+0x2ab
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!PyEval_EvalCode+0x2e
python34.dll!builtin_exec+0x1b5
python34.dll!PyCFunction_Call+0x12d
python34.dll!call_function+0x2ab
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!function_call+0x15d
python34.dll!PyObject_Call+0x61
python34.dll!ext_do_call+0x2ab
python34.dll!PyEval_EvalFrameEx+0x22fe
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!fast_function+0x14d
python34.dll!call_function+0x311
python34.dll!PyEval_EvalFrameEx+0x2259
python34.dll!PyEval_EvalCodeEx+0x65c
python34.dll!PyEval_EvalCode+0x2e
python34.dll!run_mod+0x53
python34.dll!PyRun_StringFlags+0x9c
python34.dll!PyRun_SimpleStringFlags+0x41
python34.dll!run_command+0x55
python34.dll!Py_Main+0x683
pythonw.exe!__tmainCRTStartup+0x166
kernel32.dll!BaseThreadInitThunk+0xd
ntdll.dll!RtlUserThreadStart+0x1d

System is Windows 7 64-bit, with stock x86-64 Python 3.4.0 binaries.

Incidentally, I was feeding the 'None' to winreg.SetValueEx because that is the value that winreg.EnumValue returns for zero-length binary values. This is somewhat unexpected; I'd personally prefer to get b'' in that instance.
msg215490 - (view) Author: eryksun (eryksun) Date: 2014-04-04 05:28
In Py2Reg, the REG_BINARY (3) case sets `*retDataSize = 0` when the value is None: 

http://hg.python.org/cpython/file/04f714765c13/PC/winreg.c#l766

It doesn't modify *retDataBuf. Then in PySetValueEx, PyMem_DEL is called for the uninitialized address in data:

http://hg.python.org/cpython/file/04f714765c13/PC/winreg.c#l1566

Py2Reg in this case could also set `*retDataBuf = NULL`. RegSetValueEx allows lpData to be NULL when cbData is 0. 

http://msdn.microsoft.com/en-us/library/ms724923
msg216173 - (view) Author: John Ehresman (jpe) * Date: 2014-04-14 19:00
Here's a simple patch with a test.  Oddly, the test doesn't fail before the fix is applied when run with rt.bat, but fails when run directly.
msg216637 - (view) Author: Dave Odell (dmo2118) Date: 2014-04-17 01:50
Patch works on my end.
msg220579 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-06-14 20:59
Hi. You have some trailing whitespaces in the test file (run make patchcheck or python ../Tools/scripts/patchcheck.py). Except that, looks good to me.
msg222196 - (view) Author: Roundup Robot (python-dev) Date: 2014-07-03 16:04
New changeset f2e6c33ce3e9 by Zachary Ware in branch '2.7':
Issue #21151: Fixed a segfault in the _winreg module.
http://hg.python.org/cpython/rev/f2e6c33ce3e9

New changeset 0c5a1835af91 by Zachary Ware in branch '3.4':
Issue #21151: Fixed a segfault in the winreg module.
http://hg.python.org/cpython/rev/0c5a1835af91

New changeset 21cfbcacf0d8 by Zachary Ware in branch 'default':
Closes #21151: Merge with 3.4
http://hg.python.org/cpython/rev/21cfbcacf0d8
msg222198 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-07-03 16:08
Thanks to Dave for the report, eryksun for the suggestion, and John for the patch!

I'm not sure why the test case doesn't fail on a regular test run on Python 3; it looks like BYTE *data starts out as NULL when Python is not in interactive mode, which makes no sense to me.  The test case does crash on unpatched 2.7, though, so it's a legitimate test in my view.
History
Date User Action Args
2014-07-03 16:08:31zach.waresetpriority: high -> normal
versions: + Python 2.7, Python 3.4
messages: + msg222198

assignee: zach.ware
components: + Extension Modules, - Library (Lib)
2014-07-03 16:04:14python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg222196

resolution: fixed
stage: commit review -> resolved
2014-07-03 09:38:33Claudiu.Popasetpriority: normal -> high
nosy: + zach.ware, steve.dower
2014-06-27 05:47:02Claudiu.Popasetstage: patch review -> commit review
2014-06-14 20:59:14Claudiu.Popasetversions: + Python 3.5, - Python 3.4
nosy: + Claudiu.Popa

messages: + msg220579

stage: patch review
2014-04-17 01:50:06dmo2118setmessages: + msg216637
2014-04-14 19:00:48jpesetfiles: + fix-none-value.diff

nosy: + jpe
messages: + msg216173

keywords: + patch
2014-04-04 05:28:56eryksunsetnosy: + eryksun
messages: + msg215490
2014-04-04 03:17:01dmo2118create