classification
Title: Add support for making Linux prctl(...) calls to subprocess
Type: enhancement Stage: needs patch
Components: Extension Modules, Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, gregory.p.smith, izbyshev, vstinner
Priority: normal Keywords:

Created on 2020-12-25 05:55 by gregory.p.smith, last changed 2020-12-28 04:51 by gregory.p.smith.

Messages (6)
msg383723 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-12-25 05:55
Another use of `subprocess preexec_fn=` that I've come across has been to call Linux's `prctl` in the child process before the `exec`.

`_libc.prctl(_PR_SET_PDEATHSIG, value)` for example.

Adding a linux_prctl_calls= parameter listing information about which prctl call(s) to make in the child before exec would satisfy this needed.

No need to go overboard here, this is a _very_ low level syscall.  users need to know what they're doing and will likely abstract use away from actual users via a wrapper library.  i.e: Lets not attempt to reinvent the https://pythonhosted.org/python-prctl/ interface.

Proposal:

linux_prctl_calls: Sequence[tuple]

Where every tuple indicates one prctl call.  the tuple [0] contains a bool indicating if an error returned by that prctl call should be ignored or not.  the tuple[1:6] contain the five int arguments to be passed to prctl.  If the tuple is shorter than 2 elements, the remaining values are assumed 0.

At most, a namedtuple type could be created for this purpose to allow the user to use the https://man7.org/linux/man-pages/man2/prctl.2.html argument names rather than just blindly listing a tuple.

```
namedtuple('LinuxPrctlDescription',
            field_names='check_error, option, arg2, arg3, arg4, arg5',
            defaults=(0,0,0,0))
```

This existing helps https://bugs.python.org/issue38435 deprecate preexec_fn.
msg383757 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2020-12-25 18:32
I wonder if a dedicated datatype should be created for all os-specific parameters like https://golang.org/pkg/syscall/#SysProcAttr. Popen already has way too many parameters. And prctl is a very general interface; probably 98% of prctls would never need to be called pre-exec.

(Separately, os.prctl should be created to expose prctl in all its multiplexed glory?)

(Also, but PDEATHSIG has an infamous footgun where the the signal is sent on exit of the forking thread, which is not necessarily the exit of the invoking process.)
msg383866 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-12-27 20:53
Thanks for the golang SysProcAttr reference.  The internals of _posixsubprocess have already becoming unwieldy with the abundance of args.  Such a struct/object would also fit in well there and avoid excessive C stack use.  (as izbyshev noted during the vfork work)

Most prctl uses I noticed were PDEATHSIG but I'd need to explicitly audit those.  Users don't seem to care about it's documented main thread caveat (which matches what I've seen; most programs don't use non-daemon threads and exit the main thread).

I want what we do for this to be futureproof for the syscall so that we don't wind up merely picking one feature such as PDEATHSIG to pass a flags through to and needing to add logic to support others later on, delaying the ability to use new system features.
msg383869 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-12-27 21:43
I agree that it would be nice to pack these parameters, similar to the Windows STARTUPINFO. The subprocess.Popen constructor has more and more parameter for functions executed between fork and exec:

    def __init__(self, args, bufsize=-1, executable=None,
                 stdin=None, stdout=None, stderr=None,
                 preexec_fn=None, close_fds=True,
                 shell=False, cwd=None, env=None, universal_newlines=None,
                 startupinfo=None, creationflags=0,
                 restore_signals=True, start_new_session=False,
                 pass_fds=(), *, user=None, group=None, extra_groups=None,
                 encoding=None, errors=None, text=None, umask=-1, pipesize=-1):

Parameters:

* close_fds: close()
* pass_fds: _Py_set_inheritable_async_safe()
* restore_signals: _Py_RestoreSignals()
* start_new_session: setsid()
* user: setreuid()
* group: setregid()
* extra_groups: setgroups()
* cwd: chdir()
* umask: umask()
* XXX special case: preexec_fn.

Idea of API:
-------------
preexec = subprocess.Preexec()
preexec.setsid()
preexec.chdir(path)

popen = subprocess.Popen(cmd, preexec=preexec)
popen.wait()
-------------

It would make error reporting more helpful. For example, if the path type is not bytes or str, preexec.chdir(path) call would raise an error, rather than getting an error in the complex Popen constructor.
msg383872 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2020-12-27 22:45
On Sun, Dec 27, 2020, at 14:53, Gregory P. Smith wrote:
> Most prctl uses I noticed were PDEATHSIG but I'd need to explicitly 
> audit those.  Users don't seem to care about it's documented main 
> thread caveat (which matches what I've seen; most programs don't use 
> non-daemon threads and exit the main thread).

It works great until someone refactors their process-launching code to be asynchronous. Anyway, I don't mean to bog this discussion down in the advisability and utility of PDEATHSIG. Clearly, it needs to be supported to remove even less advisable functionality.

> 
> I want what we do for this to be futureproof for the syscall so that we 
> don't wind up merely picking one feature such as PDEATHSIG to pass a 
> flags through to and needing to add logic to support others later on, 
> delaying the ability to use new system features.

The proposal right now feels like overgeneralization leading to an icky interface. It seems in spirit no different form providing a similar interface to syscall(3). At some point the interface will become so general it defeats the initial purpose of introduction, to disallow arbitrary code execution before execve. There will always be new syscalls, multiplexed into prctl/ioctl or not, that people want to make before execution. The universal workaround of a wrapper program can satisfy those on the vanguard.
msg383884 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-12-28 04:51
Felt and understood.

The plethora of things to do between (v)fork+exec really makes me wish for a "little" eBPF interpreter rather than needing so much specific plumbing.  But that'd have the same problem as preexec_fn: in absence of something that declares an operation safe or not, it'd open the door to unsafe and leave us in no better state than preexec_fn.
History
Date User Action Args
2020-12-28 04:51:23gregory.p.smithsetmessages: + msg383884
2020-12-27 22:45:48benjamin.petersonsetmessages: + msg383872
2020-12-27 21:43:25vstinnersetnosy: + vstinner
messages: + msg383869
2020-12-27 20:53:43gregory.p.smithsetmessages: + msg383866
2020-12-26 09:54:00izbyshevsetnosy: + izbyshev
2020-12-25 18:32:07benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg383757
2020-12-25 05:55:16gregory.p.smithcreate