homepage

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.

classification
Title: Python 2.7: make private file descriptors non inheritable
Type: resource usage Stage:
Components: Versions: Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, gregory.p.smith, martin.panter, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2016年04月15日 12:59 by vstinner, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
set_inheritable.patch vstinner, 2016年04月15日 12:58 review
set_inheritable-2.patch vstinner, 2016年04月19日 12:33 review
Messages (5)
msg263488 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016年04月15日 12:58
By default, subprocess.Popen doesn't close file descriptors (close_fds=False by default) and so a child process inherit all file descriptors open by the parent process. See the PEP 446 for the long rationale.
I propose to partially backport the PEP 446: only make *private* file descriptors non-inheritable. The private file descriptor of os.urandom() is already marked as non-inheritable.
To be clear: my patch doesn't affect applications using fork(). Only child processes created with fork+exec (ex: using subprocess) are impacted. Since modified file descriptors are private (not accessible from the Python scope), I don't think that the change can cause any backward compatibility issue.
I'm not 100% sure that it's worth to take the risk of introducing bugs (backward incompatible change?), since it looks like very few users complained of leaked file descriptors. I'm not sure that the modified functions contain sensitive file descriptors.
--
I chose to add Python/fileutils.c (and Include/fileutils.h) to add the new functions:
 /* Try to make the file descriptor non-inheritable. Ignore errors. */
 PyAPI_FUNC(void) _Py_try_set_non_inheritable(int fd);
 /* Wrapper to fopen() which tries to make the file non-inheritable on success.
 Ignore errors on trying to set the file descriptor non-inheritable. */
 PyAPI_FUNC(FILE*) _Py_fopen(const char *path, const char *mode);
I had to modify PCbuild/pythoncore.vcxproj and Makefile.pre.in to add fileutils.c/h. I chose to mimick Python 3 Python/fileutils.c. Tell me if you prefer to put the new functions in an existing file (I don't see where such function should be put).
File descriptors made non inheritable:
* linuxaudiodev.open()
* mmap.mmap(fd, 0): this function duplicates the file descriptor, the duplicated file descriptor is set to non-inheritable
* mmap.mmap(-1, ...): on platforms without MAP_ANONYMOUS, the function opens /dev/zero, its file descriptor is set to non-inheritable
* ossaudiodev.open()
* ossaudiodev.openmixer()
* select.epoll()
* select.kqueue()
* sunaudiodev.open()
Other functions using fopen() have been modified to use _Py_fopen(): the file is closed a few lines below and not returned to the Python scope.
The patch also reuses _Py_try_set_non_inheritable(fd) in Modules/posixmodule.c and Python/random.c.
Not modified:
* _hotshot: the file descriptor can get retrieved with _hotshot.ProfilerType.fileno().
* find_module() of Python/import.c: imp.find_module() uses the FILE* to create a Python file object which is returned
Note: I don't think that os.openpty() should be modified to limit the risk of breaking the backward compatibility.
This issue is a much more generic issue than the change #10897.
msg263536 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年04月16日 00:52
It looks like some of these do make the file descriptor accessible to the Python user. The following are from the RST documentation:
oss_audio_device.fileno()
oss_mixer_device.fileno()
epoll.fileno()
kqueue.fileno()
audio device.fileno()
I did not find any documentation for the "linuxaudiodev" module, but it also seems to supply a public fileno() method:
$ sudo modprobe snd-pcm-oss
$ python2
Python 2.7.11 (default, Dec 6 2015, 15:43:46) 
[GCC 5.2.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import linuxaudiodev
>>> a = linuxaudiodev.open("w")
>>> a.fileno()
3
Unless there is demand for making these cases non-inheritable, it might be safest to leave them be. However it looks like the mmap file descriptor could be internal (though I’m not an expert on that module to be sure). So you might be okay with the mmap change.
msg263734 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016年04月19日 12:33
> It looks like some of these do make the file descriptor accessible to the Python user. (...)
Oh right, I reverted changes on these modules.
What do you think of the patch version 2?
Changes since patch version 1:
* Revert changes in linuxaudio, oss, select, etc. modules
* Fix _Py_try_set_non_inheritable(): *set* the FD_CLOEXEC flag, don't clear it!
* Optimize _Py_try_set_non_inheritable(): avoid fcntl() if possible (backport issue #26770)
msg263788 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年04月20日 02:12
The patch looks reasonable as far as I understand it. I don’t know what the consequences of adding new PyAPI_FUNC() symbols to 2.7 is, or changing the Windows build files, so can’t really comment on those aspects though.
I left some questions about porting _Py_open() and _Py_dup().
msg282446 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016年12月05日 17:43
Sorry, I loose interest on this backport. The patch is large, the risk of regression is high, the goal is non-obvious. I'm not sure that such bugfix is still useful since Python 2.7 is old and developers know how to workaround Python 2.7 limitations.
The problem of non-inheritable file descriptors is that it requires to modify a lot of code, not only a few functions.
For all these reasons, I prefer to close the issue as WONTFIX.
History
Date User Action Args
2022年04月11日 14:58:29adminsetgithub: 70956
2016年12月05日 17:43:31vstinnersetstatus: open -> closed
resolution: wont fix
messages: + msg282446
2016年05月07日 08:13:56martin.panterlinkissue19444 superseder
2016年04月20日 02:12:02martin.pantersetmessages: + msg263788
2016年04月19日 12:33:18vstinnersetfiles: + set_inheritable-2.patch

messages: + msg263734
2016年04月16日 00:52:07martin.pantersetmessages: + msg263536
2016年04月15日 19:30:06ned.deilysetnosy: + gregory.p.smith
2016年04月15日 12:59:01vstinnercreate

AltStyle によって変換されたページ (->オリジナル) /