New submission from STINNER Victor:

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.

----------
files: set_inheritable.patch
keywords: patch
messages: 263488
nosy: benjamin.peterson, haypo, martin.panter, serhiy.storchaka
priority: normal
severity: normal
status: open
title: Python 2.7: make private file descriptors non inheritable
type: resource usage
versions: Python 2.7
Added file: http://bugs.python.org/file42471/set_inheritable.patch

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26769>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to