classification
Title: Guard against invalid file handles in os functions
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, eryksun, iritkatriel, larry, pitrou, sbt
Priority: normal Keywords:

Created on 2012-07-06 14:16 by larry, last changed 2020-11-18 14:13 by eryksun. This issue is now closed.

Messages (8)
msg164722 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-06 14:16
#15261 shows us that Windows can crash if you pass in an invalid file handle to Windows POSIX-y functions.  We should ensure that functions which accept path-as-an-int-fd guard against this where necessary.

I propose a macro, something like

#ifdef MS_WINDOWS
#define FD_GUARD(fd) (_PyVerify_fd(fd) ? (fd) : INVALID_HANDLE_VALUE)
#else
#define FD_GUARD(fd) (fd)
#endif
msg164724 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-06 14:26
Which other functions are you thinking about?
msg164725 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-06 14:47
Windows will also crash if you pass INVALID_HANDLE_VALUE (which is not a file descriptor) to crt functions...
How did you want to use this macro?
msg164727 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-06 15:10
Antoine: all the functions enumerated in os.supports_fd.  Note that the set of such functions may in fact just be os.stat which is already fixed.

Amaury: If you read the checkin that fixes this problem ( 62b9bfbc3356 ) it actually deliberately passes in INVALID_HANDLE_VALUE to win32_fstat.  I admit that's kind of where I lost interest.  I'm not really a Windows developer anymore, so maybe somebody else should do it.  So I have given myself a demotion; I am no longer assigned the bug.
msg164729 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-06 15:21
> Antoine: all the functions enumerated in os.supports_fd.  Note that the 
> set of such functions may in fact just be os.stat which is already fixed.

As far as I can tell, it is:

>>> os.supports_fd
{<built-in function stat>}
msg164731 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-07-06 15:54
Every use of _get_osfhandle() should be guarded by _Py_VerifyFd().  

Grepping through the source it seems that that is now true, but we could instead use

  #define _PY_GET_OSFHANDLE(fd) _Py_VerifyFd(fd) ? _get_osfhandle(fd) : INVALID_HANDLE_VALUE
msg381326 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-11-18 11:54
I don't see any _Py_VerifyFd in the code, is this out of date?
msg381340 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-11-18 14:13
> I don't see any _Py_VerifyFd in the code, is this out of date?

Yes, this issue is out of date. The underlying issue is that the C runtime in Windows automatically checks various parameters and calls a registered handler for invalid parameters, for which the default handler terminates the process. Python 3.5+ uses a new C runtime (ucrt), which supports setting a handler for just the current thread, so this issue is addressed more generally nowadays by the macros _Py_BEGIN_SUPPRESS_IPH and _Py_END_SUPPRESS_IPH.
History
Date User Action Args
2020-11-18 14:13:55eryksunsetstatus: open -> closed

nosy: + eryksun
messages: + msg381340

resolution: out of date
stage: needs patch -> resolved
2020-11-18 11:54:13iritkatrielsetnosy: + iritkatriel
messages: + msg381326
2012-07-06 15:54:48sbtsetmessages: + msg164731
2012-07-06 15:21:35pitrousetmessages: + msg164729
2012-07-06 15:10:41larrysetassignee: larry ->
messages: + msg164727
2012-07-06 14:47:09amaury.forgeotdarcsetmessages: + msg164725
2012-07-06 14:26:30pitrousetnosy: + pitrou
messages: + msg164724
2012-07-06 14:16:36larrycreate