Issue 19050: [Windows] fflush called on pointer to potentially closed file (original) (raw)

Created on 2013-09-19 12:58 by damiro, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (18)
msg198063 - (view)	Author: Daniel Rohlfing (damiro)	Date: 2013-09-19 12:58
This snippet let my interpreter crash immediately: > import sys, io > io.open(sys.stdout.fileno(), 'wb') > fd.close() > sys.stdout.write("now writing on stdout will cause a crash") That's happened on Python 2.7.5 (default, May 15 2013, 22:43:36) [MSC v.1500 32 bit (Intel)] on win32 Windows 7 SP1 x64 The same code let Python 3.3.2 throw an exception like this: Exception OSError: OSError(9, 'Bad file descriptor') in <_io.TextIOWrapper name='' mode='w' encoding='cp850'> ignored
msg198064 - (view)	Author: Daniel Rohlfing (damiro)	Date: 2013-09-19 13:00
the correct snippet is: > fd = io.open(sys.stdout.fileno(), 'wb') > fd.close() > sys.stdout.write("now writing on stdout will cause a crash")
msg198164 - (view)	Author: Antoine Pitrou (pitrou) *	Date: 2013-09-20 20:16
I don't get a crash under Linux. Perhaps this is a Windows-specific thing.
msg198187 - (view)	Author: Tim Golden (tim.golden) *	Date: 2013-09-21 07:09
I can confirm that 2.7.2 hard-crashes as described on Windows. I'm not sure if I have the wherewithal to build 2.7 on this laptop to see if it's fixed in tip. 3.3 simply raises an IOError.
msg198243 - (view)	Author: Tim Peters (tim.peters) *	Date: 2013-09-22 02:43
Here with 2.7.5 on Windows (Vista): C:\Python27>python.exe Python 2.7.5 (default, May 15 2013, 22:43:36) [MSC v.1500 32 bit (Intel)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import io, sys >>> fd = io.open(sys.stdout.fileno(), 'wb') >>> fd.close() >>> sys.stdout.write("now writing on stdout will cause a crash") At this point Windows pops up a box saying "python.exe has stopped working". I don't have a debug build available, and all I can find out is that it's a memory error in this line, somewhere in MS's C libraries: and dword ptr ds:[718DD7A0h],0 So, ya, it's a crash ;-)
msg236026 - (view)	Author: Mark Lawrence (BreamoreBoy) *	Date: 2015-02-15 07:38
Is this still an issue with later versions of 2.7? I'm sorry, I can't try this myself as I no longer run 2.7.
msg236067 - (view)	Author: STINNER Victor (vstinner) *	Date: 2015-02-15 20:23
> I'm sorry, I can't try this myself as I no longer run 2.7. You should install Python 2.7 if you want to work on Python 2.7 only issue. It's not so hard to install Python, especially on Windows (this issue looks to be specific to Windows).
msg240519 - (view)	Author: Zachary Ware (zach.ware) *	Date: 2015-04-12 01:28
Still an issue in 2.7.10rc0+. Here's a couple different reproducers that come closer to the heart of the matter: """ >>> import os [43913 refs] >>> os.close(1) [43913 refs] >>> input() 1 [43915 refs] """ """ >>> import os [43913 refs] >>> f = file('test', 'wb') [43921 refs] >>> os.close(f.fileno()) [43921 refs] >>> f.flush() [43921 refs] >>> f.write('test') [43921 refs] >>> f.flush() """ The problem appears to be calling fflush on a pointer to a closed file. In the first reproducer, this happens in myreadline.c, the second in fileobject.c. I was interested enough to track it down; I'm not motivated enough to fix it since it appears to be broken only in 2.7.
msg240520 - (view)	Author: Zachary Ware (zach.ware) *	Date: 2015-04-12 01:29
On sudden inspiration, here's an even simpler reproducer: """ >>> import os [43913 refs] >>> os.close(2) """
msg240521 - (view)	Author: Zachary Ware (zach.ware) *	Date: 2015-04-12 01:34
...and that one does crash 3.4, so I'm a bit more interested again. I'll try to look at this at the sprints.
msg240533 - (view)	Author: Eryk Sun (eryksun) *	Date: 2015-04-12 03:54
PyOS_StdioReadline does an unchecked fflush(sys_stdout) and fflush(stderr). It only 'crashes' (the processes is intentionally killed by the CRT) if there's buffered data to flush. Since it prints the prompt to stderr, Zachary's example won't crash if sys.ps1 and sys.ps2 are empty strings. (Does anyone know why the prompt goes to stderr? readline on Linux uses stdout.) These calls should be guarded by _PyVerify_fd. On a related vein, in I suggested adding a custom closer for file objects that checks _PyVerify_fd, but the check could also be added directly to close_the_file in fileobject.c. Here's the stack trace for 2.7 (it's similar for 3.4): C:\Program Files\Python27>cdb -xi ld python Microsoft (R) Windows Debugger Version 6.12.0002.633 AMD64 Copyright (c) Microsoft Corporation. All rights reserved. CommandLine: python Symbol search path is: symsrv*symsrv.dll*C:\Symbols*http://msdl.microsoft.com/download/symbols Executable search path is: (1078.11ec): Break instruction exception - code 80000003 (first chance) ntdll!LdrpDoDebuggerBreak+0x30: 00000000`76e8cb70 cc int 3 0:000> g Python 2.7.9 (default, Dec 10 2014, 12:28:03) [MSC v.1500 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.close(2) ntdll!ZwTerminateProcess+0xa: 00000000`76e3157a c3 ret 0:000> kc Call Site ntdll!ZwTerminateProcess KERNELBASE!TerminateProcess MSVCR90!invoke_watson MSVCR90!invalid_parameter MSVCR90!write MSVCR90!flush MSVCR90!fflush_nolock MSVCR90!fflush python27!PyOS_StdioReadline python27!PyOS_Readline python27!tok_nextc python27!tok_get python27!PyTokenizer_Get python27!parsetok python27!PyParser_ParseFileFlagsEx python27!PyParser_ASTFromFile python27!PyRun_InteractiveOneFlags python27!PyRun_InteractiveLoopFlags python27!PyRun_AnyFileExFlags python27!Py_Main python!__tmainCRTStartup kernel32!BaseThreadInitThunk ntdll!RtlUserThreadStart
msg240592 - (view)	Author: STINNER Victor (vstinner) *	Date: 2015-04-13 07:22
> This snippet let my interpreter crash immediately Well, I don't understand the issue. If you write a bug, Python crashes. Ok, so? Please fix your bug. Don't expect Python from recovering from any bug. The io stack of Python 3 is safer because it has a direct control on file descriptors, whereas Python 2 io stack relies on the C library "stdio.h". I suggest to close this issue as "wontfix" and add it to the long list of "Bugs in the C stdio (used by the Python I/O)": https://haypo-notes.readthedocs.org/python.html#bugs-in-the-c-stdio-used-by-the-python-i-o
msg240593 - (view)	Author: STINNER Victor (vstinner) *	Date: 2015-04-13 07:23
Python 3 raises an exception as expected, so I remove it from the Versions field. This issue is specific to Python 2, and it looks to be only reproductible on Windows. It's probably a bug in the C library (stdio.h).
msg240602 - (view)	Author: Eryk Sun (eryksun) *	Date: 2015-04-13 11:46
The default (and standards-violating) behavior of the Windows CRT is to kill the process for a bad file descriptor, instead of just setting errno to EBADF. To work around this PyOS_StdioReadline needs to to check _Py_Verify_fd before calling fflush or writing to stderr. A similar check was added to my_fgets in 3.2.5 (see issue 14433), but it wasn't backported to Python 2. The REPL in 3.x still uses C FILE streams, so this problem absolutely affects 3.x. (Except 3.5.0a2 and 3.5.0a3 have a hack that hides the problem. The hack has since been removed, so the problem has returned out of hiding in recent builds.) Previously I omitted the stack trace for brevity because it's virtually identical to 2.7, but here it is as justification for adding 3.x back to the issue's versions field. C:\Program Files\Python34>cdb -xi ld python Microsoft (R) Windows Debugger Version 6.12.0002.633 AMD64 Copyright (c) Microsoft Corporation. All rights reserved. CommandLine: python Symbol search path is: symsrv*symsrv.dll*C:\Symbols*http://msdl.microsoft.com/download/symbols Executable search path is: (1184.11f0): Break instruction exception - code 80000003 (first chance) ntdll!LdrpDoDebuggerBreak+0x30: 00000000`76e8cb70 cc int 3 0:000> bp ntdll!NtTerminateProcess 0:000> g Python 3.4.2 (v3.4.2:ab2c023a9432, Oct 6 2014, 22:16:31) [MSC v.1600 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.close(2) Breakpoint 0 hit ntdll!NtTerminateProcess: 00000000`76e31570 4c8bd1 mov r10,rcx 0:000> kc Call Site ntdll!NtTerminateProcess KERNELBASE!TerminateProcess MSVCR100!invalid_parameter MSVCR100!invalid_parameter_noinfo MSVCR100!write MSVCR100!flush MSVCR100!fflush_nolock MSVCR100!fflush python34!PyOS_StdioReadline python34!PyOS_Readline python34!tok_nextc python34!tok_get python34!PyTokenizer_Get python34!parsetok python34!PyParser_ParseFileObject python34!PyParser_ASTFromFileObject python34!PyRun_InteractiveOneObject python34!PyRun_InteractiveLoopFlags python34!PyRun_AnyFileExFlags python34!run_file python34!Py_Main python!__tmainCRTStartup kernel32!BaseThreadInitThunk ntdll!RtlUserThreadStart Additionally, here's a trace that demonstrates the silent handler hack that's present in 3.5.0a2: C:\Program Files\Python35>cdb -xi ld python Microsoft (R) Windows Debugger Version 6.12.0002.633 AMD64 Copyright (c) Microsoft Corporation. All rights reserved. CommandLine: python Symbol search path is: symsrv*symsrv.dll*C:\Symbols*http://msdl.microsoft.com/download/symbols Executable search path is: (1040.113c): Break instruction exception - code 80000003 (first chance) ntdll!LdrpDoDebuggerBreak+0x30: 00000000`76e8cb70 cc int 3 0:000> bp ucrtbase!set_thread_local_invalid_parameter_handler 0:000> bp python35!_silent_invalid_parameter_handler 0:000> g With this hack in place, new_threadstate unconditionally sets the invalid parameter handler to _silent_invalid_parameter_handler (again, this has since been removed): Breakpoint 0 hit ucrtbase!set_thread_local_invalid_parameter_handler: 000007fe`e47c8740 4053 push rbx 0:000> kc Call Site ucrtbase!set_thread_local_invalid_parameter_handler python35!new_threadstate python35!_Py_InitializeEx_Private python35!Py_Main python!__scrt_common_main_seh kernel32!BaseThreadInitThunk ntdll!RtlUserThreadStart 0:000> g Python 3.5.0a2 (v3.5.0a2:0337bd7ebcb6, Mar 8 2015, 07:17:31) [MSC v.1900 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.close(2) Breakpoint 1 hit python35!_silent_invalid_parameter_handler: 00000000`5f6e9d60 c20000 ret 0 0:000> kc Call Site python35!_silent_invalid_parameter_handler ucrtbase!invalid_parameter ucrtbase!write ucrtbase!_acrt_stdio_flush_nolock ucrtbase!fflush_nolock ucrtbase!fflush python35!PyOS_StdioReadline python35!PyOS_Readline python35!tok_nextc python35!tok_get python35!parsetok python35!PyParser_ASTFromFileObject python35!PyRun_InteractiveOneObject python35!PyRun_InteractiveLoopFlags python35!PyRun_AnyFileExFlags python35!run_file python35!Py_Main python!__scrt_common_main_seh kernel32!BaseThreadInitThunk ntdll!RtlUserThreadStart I think for 3.5 the affected code needs to be bracketed by _Py_BEGIN_SUPPRESS_IPH and _Py_END_SUPPRESS_IPH. This works on my personal build. See issue 23524 for more details regarding the new macros.
msg240732 - (view)	Author: A.M. Kuchling (akuchling) *	Date: 2015-04-13 20:25
Here's a patch that adds the necessary _PyVerify_fd checking for 3.4. It won't apply to 2.7 (too many changes between 2.7 and 3.4), and applies to 3.5 but does nothing because of the new Invalid Parameter Handler introduced into 3.5. It doesn't fix the problem completely: there is then a new crash in sysmodule.c/sys_write(). Unfortunately I didn't manage to write a test that replicates this whole issue and fails if Python crashes.
msg240859 - (view)	Author: STINNER Victor (vstinner) *	Date: 2015-04-14 08:19
> The default (and standards-violating) behavior of the Windows CRT is to kill the process for a bad file descriptor, instead of just setting errno to EBADF. Ah, if the issue is the behaviour of the MSVCRT on EBADF, Steve Dower can help you. He wrote the _Py_BEGIN_SUPPRESS_IPH/_Py_END_SUPPRESS_IPH thing for Python 3.5 and Visual Studio 2015 (Issue #23668).
msg323485 - (view)	Author: Steve Dower (steve.dower) *	Date: 2018-08-13 16:26
As this is a fail-fast and not an uncontrolled crash, I vote to close as wontfix. This only applies to security-fix versions, and it is not exploitable.
msg323655 - (view)	Author: Tal Einat (taleinat) *	Date: 2018-08-17 12:10
I agree this should probably be ignored for security-only branches. However, AFAIK 2.7 is still in bugfix status, not security-only. If there's interest, I'm happy to build with AMK's fix on 2.7 on Windows to see if it works, and if so provide a PR.