classification
Title: [Python 2, Windows] fflush called on pointer to potentially closed file
Type: crash Stage:
Components: Interpreter Core, Windows Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, akuchling, damiro, eryksun, haypo, pitrou, steve.dower, tim.golden, tim.peters, zach.ware
Priority: critical Keywords: patch

Created on 2013-09-19 12:58 by damiro, last changed 2015-04-14 08:19 by haypo.

Files
File name Uploaded Description Edit
issue19050.patch akuchling, 2015-04-13 20:25
Messages (16)
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='<stdout>' 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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 (haypo) * (Python committer) 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) * (Python committer) 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]
<crash>
"""

"""
>>> 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()
<crash>
"""

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) * (Python committer) Date: 2015-04-12 01:29
On sudden inspiration, here's an even simpler reproducer:

"""
>>> import os
[43913 refs]
>>> os.close(2)
<crash>
"""
msg240521 - (view) Author: Zachary Ware (zach.ware) * (Python committer) 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 msg237845 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 (haypo) * (Python committer) 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 (haypo) * (Python committer) 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) * (Python committer) 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 (haypo) * (Python committer) 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).
History
Date User Action Args
2015-04-14 08:19:45hayposetmessages: + msg240859
2015-04-13 20:25:23akuchlingsetfiles: + issue19050.patch

nosy: + akuchling
messages: + msg240732

keywords: + patch
2015-04-13 11:46:44eryksunsetmessages: + msg240602
versions: + Python 3.4, Python 3.5
2015-04-13 07:23:47hayposetmessages: + msg240593
versions: - Python 3.4, Python 3.5
2015-04-13 07:22:52hayposetmessages: + msg240592
title: fflush called on pointer to potentially closed file -> [Python 2, Windows] fflush called on pointer to potentially closed file
2015-04-12 03:54:49eryksunsetnosy: + eryksun
messages: + msg240533
2015-04-12 01:34:48zach.waresetpriority: high -> critical

title: crash while writing to a closed file descriptor -> fflush called on pointer to potentially closed file
messages: + msg240521
versions: + Python 3.4, Python 3.5
2015-04-12 01:29:55zach.waresetmessages: + msg240520
2015-04-12 01:28:28zach.waresetmessages: + msg240519
2015-02-15 20:23:20hayposetmessages: + msg236067
2015-02-15 16:28:32brian.curtinsetnosy: - brian.curtin
2015-02-15 07:38:38BreamoreBoysetnosy: + BreamoreBoy, zach.ware, steve.dower
messages: + msg236026
2013-09-24 21:56:52hayposetnosy: + haypo
2013-09-22 02:43:39tim.peterssetmessages: + msg198243
2013-09-21 07:10:41rhettingersetpriority: normal -> high
2013-09-21 07:09:43tim.goldensetmessages: + msg198187
2013-09-20 20:16:41pitrousetnosy: + tim.golden, brian.curtin, pitrou, tim.peters
messages: + msg198164
components: + Interpreter Core, Windows, - Library (Lib)
2013-09-19 13:00:44damirosetmessages: + msg198064
2013-09-19 12:58:29damirocreate