On 12/30/18 12:44 AM, Michał Górny wrote: > On Sat, 2018-12-29 at 04:49 -0800, Zac Medico wrote: >> In order to prevent failed unshare calls from corrupting the state >> of an essential process, validate the relevant unshare call in a >> short-lived subprocess. An unshare call is considered valid if it >> successfully executes in a short-lived subprocess. >> >> Bug: https://bugs.gentoo.org/673900 >> Signed-off-by: Zac Medico <zmed...@gentoo.org> >> --- >> lib/portage/process.py | 130 +++++++++++++++++++++++++++++++++-------- >> 1 file changed, 106 insertions(+), 24 deletions(-) >> >> diff --git a/lib/portage/process.py b/lib/portage/process.py >> index ce3e42a8f..4bd6a4192 100644 >> --- a/lib/portage/process.py >> +++ b/lib/portage/process.py >> @@ -6,6 +6,7 @@ >> import atexit >> import errno >> import fcntl >> +import multiprocessing >> import platform >> import signal >> import socket >> @@ -338,11 +339,29 @@ def spawn(mycommand, env=None, opt_name=None, >> fd_pipes=None, returnpid=False, >> fd_pipes[1] = pw >> fd_pipes[2] = pw >> >> - # This caches the libc library lookup in the current >> - # process, so that it's only done once rather than >> + # This caches the libc library lookup and _unshare_validator results >> + # in the current process, so that it's only done once rather than >> # for each child process. >> + unshare_flags = 0 >> if unshare_net or unshare_ipc or unshare_mount or unshare_pid: >> - find_library("c") >> + # from /usr/include/bits/sched.h >> + CLONE_NEWNS = 0x00020000 >> + CLONE_NEWIPC = 0x08000000 >> + CLONE_NEWPID = 0x20000000 >> + CLONE_NEWNET = 0x40000000 >> + >> + if unshare_net: >> + unshare_flags |= CLONE_NEWNET >> + if unshare_ipc: >> + unshare_flags |= CLONE_NEWIPC >> + if unshare_mount: >> + # NEWNS = mount namespace >> + unshare_flags |= CLONE_NEWNS >> + if unshare_pid: >> + # we also need mount namespace for slave /proc >> + unshare_flags |= CLONE_NEWPID | CLONE_NEWNS >> + >> + _unshare_validator.instance.validate(unshare_flags) >> >> # Force instantiation of portage.data.userpriv_groups before the >> # fork, so that the result is cached in the main process. >> @@ -358,7 +377,7 @@ def spawn(mycommand, env=None, opt_name=None, >> fd_pipes=None, returnpid=False, >> _exec(binary, mycommand, opt_name, fd_pipes, >> env, gid, groups, uid, umask, cwd, >> pre_exec, close_fds, >> unshare_net, unshare_ipc, >> unshare_mount, unshare_pid, >> - cgroup) >> + unshare_flags, cgroup) >> except SystemExit: >> raise >> except Exception as e: >> @@ -430,7 +449,7 @@ def spawn(mycommand, env=None, opt_name=None, >> fd_pipes=None, returnpid=False, >> def _exec(binary, mycommand, opt_name, fd_pipes, >> env, gid, groups, uid, umask, cwd, >> pre_exec, close_fds, unshare_net, unshare_ipc, unshare_mount, >> unshare_pid, >> - cgroup): >> + unshare_flags, cgroup): >> >> """ >> Execute a given binary with options >> @@ -466,6 +485,8 @@ def _exec(binary, mycommand, opt_name, fd_pipes, >> @type unshare_mount: Boolean >> @param unshare_pid: If True, PID ns will be unshared from the spawned >> process >> @type unshare_pid: Boolean >> + @param unshare_flags: Flags for the unshare(2) function >> + @type unshare_flags: Integer >> @param cgroup: CGroup path to bind the process to >> @type cgroup: String >> @rtype: None >> @@ -527,26 +548,14 @@ def _exec(binary, mycommand, opt_name, fd_pipes, >> if filename is not None: >> libc = LoadLibrary(filename) >> if libc is not None: >> - # from /usr/include/bits/sched.h >> - CLONE_NEWNS = 0x00020000 >> - CLONE_NEWIPC = 0x08000000 >> - CLONE_NEWPID = 0x20000000 >> - CLONE_NEWNET = 0x40000000 >> - >> - flags = 0 >> - if unshare_net: >> - flags |= CLONE_NEWNET >> - if unshare_ipc: >> - flags |= CLONE_NEWIPC >> - if unshare_mount: >> - # NEWNS = mount namespace >> - flags |= CLONE_NEWNS >> - if unshare_pid: >> - # we also need mount namespace for >> slave /proc >> - flags |= CLONE_NEWPID | CLONE_NEWNS >> - >> try: >> - if libc.unshare(flags) != 0: >> + errno_value = >> _unshare_validator.instance.validate(unshare_flags) >> + if errno_value != 0: >> + writemsg("Unable to unshare: >> %s\n" % ( >> + >> errno.errorcode.get(errno_value, '?')), >> + noiselevel=-1) >> + raise AttributeError('unshare') >> + if libc.unshare(unshare_flags) != 0: >> writemsg("Unable to unshare: >> %s\n" % ( >> >> errno.errorcode.get(ctypes.get_errno(), '?')), >> noiselevel=-1) >> @@ -626,6 +635,79 @@ def _exec(binary, mycommand, opt_name, fd_pipes, >> # And switch to the new process. >> os.execve(binary, myargs, env) >> >> + >> +class _unshare_validator(object): >> + """ >> + In order to prevent failed unshare calls from corrupting the state >> + of an essential process, validate the relevant unshare call in a >> + short-lived subprocess. An unshare call is considered valid if it >> + successfully executes in a short-lived subprocess. >> + """ >> + >> + instance = None >> + >> + def __init__(self): >> + self._results = {} >> + >> + def validate(self, flags): >> + """ >> + Validate unshare with the given flags. Results are cached. >> + >> + @rtype: int >> + @returns: errno value, or 0 if no error occurred. >> + """ >> + >> + if flags == 0: >> + return 0 >> + >> + result = self._results.get(flags) >> + if result is not None: >> + return result >> + >> + filename = find_library("c") >> + if filename is None: >> + return errno.ENOTSUP >> + >> + libc = LoadLibrary(filename) >> + if libc is None: >> + return errno.ENOTSUP >> + >> + parent_pipe, subproc_pipe = multiprocessing.Pipe(duplex=False) >> + >> + proc = multiprocessing.Process( >> + target=self._validator_subproc, >> + args=(libc.unshare, flags, subproc_pipe)) >> + proc.start() >> + subproc_pipe.close() >> + >> + result = parent_pipe.recv() >> + parent_pipe.close() >> + proc.join() >> + >> + self._results[flags] = result >> + return result >> + >> + def _validator_subproc(self, unshare, flags, subproc_pipe): >> + """ >> + Perform validation, and send results to parent process. >> + >> + @param unshare: unshare function >> + @type unshare: callable >> + @param flags: unshare flags >> + @type flags: int >> + @param subproc_pipe: connection to parent process >> + @type subproc_pipe: multiprocessing.Connection >> + """ >> + if unshare(flags) != 0: >> + subproc_pipe.send(ctypes.get_errno()) >> + else: >> + subproc_pipe.send(0) >> + subproc_pipe.close() >> + >> + >> +_unshare_validator.instance = _unshare_validator() >> + >> + >> def _setup_pipes(fd_pipes, close_fds=True, inheritable=None): >> """Setup pipes for a forked process. >> > > Can't we detect the failure in normally spawned ebuild process, > and restart it with unsharing disabled? Possibly cache the result to > avoid having to restart everything afterwards.
We'd need an IPC mechanism to communicate the failure to the parent process so that it could retry. However, _exec is intended to exec an arbitrary child process, so it's not an appropriate place for IPC. -- Thanks, Zac
signature.asc
Description: OpenPGP digital signature
