From 4a9da60644fad113881289b4ca9cbb008c451344 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Tue, 8 Oct 2019 09:32:54 +0200 Subject: [PATCH 1/2] mock: fix broken rpmbuildstate Previously, when something in 'rebuild_package()' method threw an expection the state.finish(rpmbuildstate) wasn't called, and this caused another exception in finally block where we tried to state.finish(buildstate): mockbuild.exception.StateError: state finish mismatch: current: rpmbuild dummy-pkg-20191008_0834-0.src.rpm, state: build phase for dummy-pkg-20191008_0834-0.src.rpm This exception was not handled at all, so uid_manager didn't have a chance to raise privileges again by restorePrivs() - and the exception bubbled up to main(). In the meantime, the cleanup_on_failure=True handler was executed in rebuild_generic() - but without administrator privileges so it failed. Fixes: #349 --- mock/py/mockbuild/backend.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mock/py/mockbuild/backend.py b/mock/py/mockbuild/backend.py index 52624e2fa..830c70d9d 100644 --- a/mock/py/mockbuild/backend.py +++ b/mock/py/mockbuild/backend.py @@ -297,13 +297,17 @@ def build(self, srpm, timeout, check=True, spec=None): buildsetup_finished = True rpmbuildstate = "rpmbuild %s" % baserpm - self.state.start(rpmbuildstate) # tell caching we are building self.plugins.call_hooks('prebuild') # intentionally we do not call bootstrap hook here - it does not have sense - results = self.rebuild_package(spec_path, timeout, check, dynamic_buildreqs) + try: + self.state.start(rpmbuildstate) + results = self.rebuild_package(spec_path, timeout, check, dynamic_buildreqs) + finally: + self.state.finish(rpmbuildstate) + # In the nspawn case, we retained root until here, but we # need to ensure our output files are owned by the caller's uid. # So drop them now. @@ -318,7 +322,6 @@ def build(self, srpm, timeout, check=True, spec=None): raise PkgError('No build results found') self.state.result = 'success' - self.state.finish(rpmbuildstate) finally: if not buildsetup_finished: self.state.finish(buildsetup) From 9601e90a7a6832903a59173c835e5d07ca2bdd44 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Tue, 8 Oct 2019 09:33:33 +0200 Subject: [PATCH 2/2] mock: drop two unnecessary privilege escalations At those points in code path, mock already had appropriate permissions. So I believe this was a workaround for bug fixed by previous commit. More, one of the privilege escalation calls was not taken back when bootstrap_chroot was disabled and mock seemingly worked, so it sort of confirms that this commit isn't incorrect. --- mock/py/mock.py | 2 -- mock/py/mockbuild/backend.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/mock/py/mock.py b/mock/py/mock.py index 89e951627..5f088c6ee 100755 --- a/mock/py/mock.py +++ b/mock/py/mock.py @@ -752,11 +752,9 @@ def main(): try: result = run_command(options, args, config_opts, commands, buildroot, state) finally: - buildroot.uid_manager.becomeUser(0, 0) buildroot.finalize() if bootstrap_buildroot is not None: bootstrap_buildroot.finalize() - buildroot.uid_manager.restorePrivs() return result diff --git a/mock/py/mockbuild/backend.py b/mock/py/mockbuild/backend.py index 830c70d9d..462ee16da 100644 --- a/mock/py/mockbuild/backend.py +++ b/mock/py/mockbuild/backend.py @@ -489,11 +489,9 @@ def chain(self, args, options, buildroot): except Error: build_ret_code = 1 finally: - buildroot.uid_manager.becomeUser(0, 0) buildroot.finalize() if buildroot.bootstrap_buildroot is not None: buildroot.bootstrap_buildroot.finalize() - buildroot.uid_manager.restorePrivs() except (RootError,) as e: log.warning(e.msg) failed.append(pkg)