[gentoo-portage-dev] [PATCH] EbuildIpcDaemon: fix lock permission race

[Zac Medico <zmedico@gentoo.org>]

Move ipc files to a .ipc subdirectory, with a setgid bit to
prevent a lockfile group permission race. The lockfile function
uses an appropriate open call with mode argument so that the
lockfile is created atomically with both group ownership and
group write bit.

Bug: https://bugs.gentoo.org/468990
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
 bin/ebuild-ipc.py                                | 6 +++---
 bin/phase-functions.sh                           | 4 ++--
 lib/_emerge/AbstractEbuildProcess.py             | 4 ++--
 lib/_emerge/EbuildIpcDaemon.py                   | 2 +-
 lib/portage/package/ebuild/prepare_build_dirs.py | 6 ++++++
 lib/portage/tests/ebuild/test_doebuild_spawn.py  | 1 +
 lib/portage/tests/ebuild/test_ipc_daemon.py      | 6 +++---
 7 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/bin/ebuild-ipc.py b/bin/ebuild-ipc.py
index 4999c043a..c24ba6f58 100755
--- a/bin/ebuild-ipc.py
+++ b/bin/ebuild-ipc.py
@@ -138,9 +138,9 @@ class EbuildIpc:
 
     def __init__(self):
         self.fifo_dir = os.environ["PORTAGE_BUILDDIR"]
-        self.ipc_in_fifo = os.path.join(self.fifo_dir, ".ipc_in")
-        self.ipc_out_fifo = os.path.join(self.fifo_dir, ".ipc_out")
-        self.ipc_lock_file = os.path.join(self.fifo_dir, ".ipc_lock")
+        self.ipc_in_fifo = os.path.join(self.fifo_dir, ".ipc", "in")
+        self.ipc_out_fifo = os.path.join(self.fifo_dir, ".ipc", "out")
+        self.ipc_lock_file = os.path.join(self.fifo_dir, ".ipc", "lock")
 
     def _daemon_is_alive(self):
         try:
diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
index d3221993d..5eb031805 100644
--- a/bin/phase-functions.sh
+++ b/bin/phase-functions.sh
@@ -291,10 +291,10 @@ __dyn_clean() {
 		rm -f "$PORTAGE_BUILDDIR"/.{ebuild_changed,logid,pretended,setuped,unpacked,prepared} \
 			"$PORTAGE_BUILDDIR"/.{configured,compiled,tested,packaged,instprepped} \
 			"$PORTAGE_BUILDDIR"/.die_hooks \
-			"$PORTAGE_BUILDDIR"/.ipc_{in,out,lock} \
 			"$PORTAGE_BUILDDIR"/.exit_status
 
-		rm -rf "${PORTAGE_BUILDDIR}/build-info"
+		rm -rf "${PORTAGE_BUILDDIR}/build-info" \
+			"${PORTAGE_BUILDDIR}/.ipc"
 		rm -rf "${WORKDIR}"
 		rm -f "${PORTAGE_BUILDDIR}/files"
 	fi
diff --git a/lib/_emerge/AbstractEbuildProcess.py b/lib/_emerge/AbstractEbuildProcess.py
index 1b4e7759f..6d89d40f0 100644
--- a/lib/_emerge/AbstractEbuildProcess.py
+++ b/lib/_emerge/AbstractEbuildProcess.py
@@ -249,8 +249,8 @@ class AbstractEbuildProcess(SpawnProcess):
 
     def _init_ipc_fifos(self):
 
-        input_fifo = os.path.join(self.settings["PORTAGE_BUILDDIR"], ".ipc_in")
-        output_fifo = os.path.join(self.settings["PORTAGE_BUILDDIR"], ".ipc_out")
+        input_fifo = os.path.join(self.settings["PORTAGE_BUILDDIR"], ".ipc", "in")
+        output_fifo = os.path.join(self.settings["PORTAGE_BUILDDIR"], ".ipc", "out")
 
         for p in (input_fifo, output_fifo):
 
diff --git a/lib/_emerge/EbuildIpcDaemon.py b/lib/_emerge/EbuildIpcDaemon.py
index ee6fd7658..78594ff0a 100644
--- a/lib/_emerge/EbuildIpcDaemon.py
+++ b/lib/_emerge/EbuildIpcDaemon.py
@@ -81,7 +81,7 @@ class EbuildIpcDaemon(FifoIpcDaemon):
             # write something to the pipe just before we close it, and in that
             # case the write will be lost. Therefore, try for a non-blocking
             # lock, and only re-open the pipe if the lock is acquired.
-            lock_filename = os.path.join(os.path.dirname(self.input_fifo), ".ipc_lock")
+            lock_filename = os.path.join(os.path.dirname(self.input_fifo), "lock")
             try:
                 lock_obj = lockfile(lock_filename, unlinkfile=True, flags=os.O_NONBLOCK)
             except TryAgain:
diff --git a/lib/portage/package/ebuild/prepare_build_dirs.py b/lib/portage/package/ebuild/prepare_build_dirs.py
index 659198905..9d2474fd8 100644
--- a/lib/portage/package/ebuild/prepare_build_dirs.py
+++ b/lib/portage/package/ebuild/prepare_build_dirs.py
@@ -102,6 +102,12 @@ def prepare_build_dirs(myroot=None, settings=None, cleanup=False):
             apply_secpass_permissions(
                 mysettings[dir_key], uid=portage_uid, gid=portage_gid
             )
+        # The setgid bit prevents a lockfile group permission race for bug #468990.
+        ensure_dirs(
+            os.path.join(mysettings["PORTAGE_BUILDDIR"], ".ipc"),
+            gid=portage_gid,
+            mode=0o2770,
+        )
     except PermissionDenied as e:
         writemsg(_("Permission Denied: %s\n") % str(e), noiselevel=-1)
         return 1
diff --git a/lib/portage/tests/ebuild/test_doebuild_spawn.py b/lib/portage/tests/ebuild/test_doebuild_spawn.py
index ef0ae5847..d142ec41e 100644
--- a/lib/portage/tests/ebuild/test_doebuild_spawn.py
+++ b/lib/portage/tests/ebuild/test_doebuild_spawn.py
@@ -81,6 +81,7 @@ class DoebuildSpawnTestCase(TestCase):
             settings["T"] = os.path.join(settings["PORTAGE_BUILDDIR"], "temp")
             for x in ("PORTAGE_BUILDDIR", "HOME", "T"):
                 os.makedirs(settings[x])
+            os.makedirs(os.path.join(settings["PORTAGE_BUILDDIR"], ".ipc"))
             # Create a fake environment, to pretend as if the ebuild
             # has been sourced already.
             open(os.path.join(settings["T"], "environment"), "wb").close()
diff --git a/lib/portage/tests/ebuild/test_ipc_daemon.py b/lib/portage/tests/ebuild/test_ipc_daemon.py
index e20b6fff1..0ac3d3c32 100644
--- a/lib/portage/tests/ebuild/test_ipc_daemon.py
+++ b/lib/portage/tests/ebuild/test_ipc_daemon.py
@@ -66,10 +66,10 @@ class IpcDaemonTestCase(TestCase):
 
             build_dir = EbuildBuildDir(scheduler=event_loop, settings=env)
             event_loop.run_until_complete(build_dir.async_lock())
-            ensure_dirs(env["PORTAGE_BUILDDIR"])
+            ensure_dirs(os.path.join(env["PORTAGE_BUILDDIR"], ".ipc"))
 
-            input_fifo = os.path.join(env["PORTAGE_BUILDDIR"], ".ipc_in")
-            output_fifo = os.path.join(env["PORTAGE_BUILDDIR"], ".ipc_out")
+            input_fifo = os.path.join(env["PORTAGE_BUILDDIR"], ".ipc", "in")
+            output_fifo = os.path.join(env["PORTAGE_BUILDDIR"], ".ipc", "out")
             os.mkfifo(input_fifo)
             os.mkfifo(output_fifo)
 
-- 
2.32.0