Skip to content

gh-91401: Add a failsafe way to disable vfork. #91490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1529,3 +1529,29 @@ runtime):

:mod:`shlex`
Module which provides function to parse and escape command lines.


.. _disable_vfork:

Disabling potential use of ``vfork()``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

On Linux, :mod:`subprocess` defaults to using the ``vfork()`` system call
internally when it is safe to do so rather than ``fork()``. This greatly
improves performance.

If you ever encounter a presumed highly-unusual situation where you need to
prevent ``vfork()`` from being used by Python, you can set the
:attr:`subprocess.disable_vfork_reason` attribute to a non-empty string.
Ideally one describing why and linking to a bug report explaining how to setup
an environment with code to reproduce the issue preventing it from working
properly. Without recording that information, nobody will understand when they
can unset it in your code in the future.

Setting this has no impact on use of ``posix_spawn()`` which could use
``vfork()`` within its libc implementation.

It is safe to set this attribute on older Python versions. Do not assume it
exists to be read until 3.11.

.. versionadded:: 3.11 ``disable_vfork_reason``
4 changes: 3 additions & 1 deletion Lib/multiprocessing/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,13 +446,15 @@ def _flush_std_streams():

def spawnv_passfds(path, args, passfds):
import _posixsubprocess
import subprocess
passfds = tuple(sorted(map(int, passfds)))
errpipe_read, errpipe_write = os.pipe()
try:
return _posixsubprocess.fork_exec(
args, [os.fsencode(path)], True, passfds, None, None,
-1, -1, -1, -1, -1, -1, errpipe_read, errpipe_write,
False, False, None, None, None, -1, None)
False, False, None, None, None, -1, None,
subprocess.disable_vfork_reason)
finally:
os.close(errpipe_read)
os.close(errpipe_write)
Expand Down
10 changes: 9 additions & 1 deletion Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ def _fork_exec(*args, **kwargs):
import selectors


# This is purely a failsafe. If non-empty the _posixsubprocess code will never
# consider using vfork(). While any true value will trigger this, set it to a
# description of why it is being disabled. With a link to a bug report
# explaining how to setup an environment with code to reproduce the issue this
# is working around. Otherwise nobody will understand when they can unset it.
disable_vfork_reason = ""


# Exception classes used by this module.
class SubprocessError(Exception): pass

Expand Down Expand Up @@ -1792,7 +1800,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
errpipe_read, errpipe_write,
restore_signals, start_new_session,
gid, gids, uid, umask,
preexec_fn)
preexec_fn, disable_vfork_reason)
self._child_created = True
finally:
# be sure the FD is closed no matter what
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,15 @@ class Z(object):
def __len__(self):
return 1
self.assertRaises(TypeError, _posixsubprocess.fork_exec,
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21)
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22)
# Issue #15736: overflow in _PySequence_BytesToCharpArray()
class Z(object):
def __len__(self):
return sys.maxsize
def __getitem__(self, i):
return b'x'
self.assertRaises(MemoryError, _posixsubprocess.fork_exec,
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21)
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22)

@unittest.skipUnless(_posixsubprocess, '_posixsubprocess required for this test.')
def test_subprocess_fork_exec(self):
Expand All @@ -136,7 +136,7 @@ def __len__(self):

# Issue #15738: crash in subprocess_fork_exec()
self.assertRaises(TypeError, _posixsubprocess.fork_exec,
Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21)
Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22)

@unittest.skipIf(MISSING_C_DOCSTRINGS,
"Signature information for builtins requires docstrings")
Expand Down
7 changes: 4 additions & 3 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -3107,7 +3107,7 @@ def test_fork_exec(self):
1, 2, 3, 4,
True, True,
False, [], 0, -1,
func)
func, "")
# Attempt to prevent
# "TypeError: fork_exec() takes exactly N arguments (M given)"
# from passing the test. More refactoring to have us start
Expand Down Expand Up @@ -3156,7 +3156,7 @@ def __int__(self):
1, 2, 3, 4,
True, True,
None, None, None, -1,
None)
None, "no vfork")
self.assertIn('fds_to_keep', str(c.exception))
finally:
if not gc_enabled:
Expand Down Expand Up @@ -3614,7 +3614,8 @@ def test_getoutput(self):

def test__all__(self):
"""Ensure that __all__ is populated properly."""
intentionally_excluded = {"list2cmdline", "Handle", "pwd", "grp", "fcntl"}
intentionally_excluded = {"list2cmdline", "Handle", "pwd", "grp",
"fcntl", "disable_vfork_reason"}
exported = set(subprocess.__all__)
possible_exports = set()
import types
Expand Down
7 changes: 4 additions & 3 deletions Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -751,17 +751,18 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
Py_ssize_t arg_num, num_groups = 0;
int need_after_fork = 0;
int saved_errno = 0;
int disable_vfork;

if (!PyArg_ParseTuple(
args, "OOpO!OOiiiiiiiiiiOOOiO:fork_exec",
args, "OOpO!OOiiiiiiiiiiOOOiOp:fork_exec",
&process_args, &executable_list,
&close_fds, &PyTuple_Type, &py_fds_to_keep,
&cwd_obj, &env_list,
&p2cread, &p2cwrite, &c2pread, &c2pwrite,
&errread, &errwrite, &errpipe_read, &errpipe_write,
&restore_signals, &call_setsid,
&gid_object, &groups_list, &uid_object, &child_umask,
&preexec_fn))
&preexec_fn, &disable_vfork))
return NULL;

if ((preexec_fn != Py_None) &&
Expand Down Expand Up @@ -940,7 +941,7 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
#ifdef VFORK_USABLE
/* Use vfork() only if it's safe. See the comment above child_exec(). */
sigset_t old_sigs;
if (preexec_fn == Py_None &&
if (preexec_fn == Py_None && !disable_vfork &&
!call_setuid && !call_setgid && !call_setgroups) {
/* Block all signals to ensure that no signal handlers are run in the
* child process while it shares memory with us. Note that signals
Expand Down