Ensure directories used by gProfiler to host async-profiler DSO are owned by root (#832)

Author: Jongy

gprofiler/main.py

@@ -65,7 +65,7 @@
     resource_path,
     run_process,
 )
-from gprofiler.utils.fs import escape_filename
+from gprofiler.utils.fs import escape_filename, mkdir_owned_root
 from gprofiler.utils.proxy import get_https_proxy
 
 if is_linux():
@@ -954,6 +954,7 @@ def main() -> None:
     if is_windows() or get_aws_execution_env() == "AWS_ECS_FARGATE":
         args.perf_mode = "disabled"
         args.pid_ns_check = False
+
     if args.subcommand != "upload-file":
         verify_preconditions(args, processes_to_profile)
 
@@ -1021,8 +1022,7 @@ def main() -> None:
                 )
                 sys.exit(1)
 
-        if not os.path.exists(TEMPORARY_STORAGE_PATH):
-            os.mkdir(TEMPORARY_STORAGE_PATH)
+        mkdir_owned_root(TEMPORARY_STORAGE_PATH)
 
         try:
             client_kwargs = {}

gprofiler/profilers/java.py

@@ -13,7 +13,7 @@
 from subprocess import CompletedProcess
 from threading import Event, Lock
 from types import TracebackType
-from typing import Any, Dict, Iterable, List, Optional, Sequence, Set, Type, TypeVar, Union
+from typing import Any, Dict, Iterable, List, Optional, Set, Type, TypeVar, Union
 
 import psutil
 from granulate_utils.java import (
@@ -79,7 +79,7 @@
     touch_path,
     wait_event,
 )
-from gprofiler.utils.fs import is_rw_exec_dir, safe_copy
+from gprofiler.utils.fs import is_owned_by_root, is_rw_exec_dir, mkdir_owned_root, safe_copy
 from gprofiler.utils.perf import can_i_use_perf_events
 from gprofiler.utils.process import process_comm, search_proc_maps
 
@@ -471,9 +471,10 @@ def __init__(
         # because storage_dir changes between runs.
         # we embed the async-profiler version in the path, so future gprofiler versions which use another version
         # of AP case use it (will be loaded as a different DSO)
+        self._ap_dir_base = self._find_rw_exec_dir()
+        self._ap_dir_versioned = os.path.join(self._ap_dir_base, f"async-profiler-{get_ap_version()}")
         self._ap_dir_host = os.path.join(
-            self._find_rw_exec_dir(POSSIBLE_AP_DIRS),
-            f"async-profiler-{get_ap_version()}",
+            self._ap_dir_versioned,
             "musl" if self._needs_musl_ap() else "glibc",
         )
 
@@ -498,20 +499,42 @@ def __init__(
         self._collect_meminfo = collect_meminfo
         self._include_method_modifiers = ",includemm" if include_method_modifiers else ""
 
-    def _find_rw_exec_dir(self, available_dirs: Sequence[str]) -> str:
+    def _find_rw_exec_dir(self) -> str:
         """
         Find a rw & executable directory (in the context of the process) where we can place libasyncProfiler.so
         and the target process will be able to load it.
+        This function creates the gprofiler_tmp directory as a directory owned by root, if it doesn't exist under the
+        chosen rwx directory.
+        It does not create the parent directory itself, if it doesn't exist (e.g /run).
+        The chosen rwx directory needs to be owned by root.
         """
-        for d in available_dirs:
-            full_dir = resolve_proc_root_links(self._process_root, d)
+        for d in POSSIBLE_AP_DIRS:
+            full_dir = Path(resolve_proc_root_links(self._process_root, d))
+            if not full_dir.parent.exists():
+                continue  # we do not create the parent.
+
+            if not is_owned_by_root(full_dir.parent):
+                continue  # the parent needs to be owned by root
+
+            mkdir_owned_root(full_dir)
+
             if is_rw_exec_dir(full_dir):
-                return full_dir
+                return str(full_dir)
         else:
-            raise NoRwExecDirectoryFoundError(f"Could not find a rw & exec directory out of {available_dirs}!")
+            raise NoRwExecDirectoryFoundError(
+                f"Could not find a rw & exec directory out of {POSSIBLE_AP_DIRS} for {self._process_root}!"
+            )
 
     def __enter__(self: T) -> T:
-        os.makedirs(self._ap_dir_host, 0o755, exist_ok=True)
+        # create the directory structure for executable libap, make sure it's owned by root
+        # for sanity & simplicity, mkdir_owned_root() does not support creating parent directories, as this allows
+        # the caller to absentmindedly ignore the check of the parents ownership.
+        # hence we create the structure here part by part.
+        assert is_owned_by_root(
+            Path(self._ap_dir_base)
+        ), f"expected {self._ap_dir_base} to be owned by root at this point"
+        mkdir_owned_root(self._ap_dir_versioned)
+        mkdir_owned_root(self._ap_dir_host)
         os.makedirs(self._storage_dir_host, 0o755, exist_ok=True)
 
         self._check_disk_requirements()

gprofiler/utils/fs.py

@@ -8,9 +8,10 @@
 import shutil
 from pathlib import Path
 from secrets import token_hex
+from typing import Union
 
 from gprofiler.platform import is_windows
-from gprofiler.utils import remove_path, run_process
+from gprofiler.utils import is_root, remove_path, run_process
 
 
 def safe_copy(src: str, dst: str) -> None:
@@ -23,18 +24,19 @@ def safe_copy(src: str, dst: str) -> None:
     os.rename(dst_tmp, dst)
 
 
-def is_rw_exec_dir(path: str) -> bool:
+def is_rw_exec_dir(path: Path) -> bool:
     """
     Is 'path' rw and exec?
     """
+    assert is_owned_by_root(path), f"expected {path} to be owned by root!"
+
     # randomize the name - this function runs concurrently on paths of in same mnt namespace.
-    test_script = Path(path) / f"t-{token_hex(10)}.sh"
+    test_script = path / f"t-{token_hex(10)}.sh"
 
     # try creating & writing
     try:
-        os.makedirs(path, 0o755, exist_ok=True)
         test_script.write_text("#!/bin/sh\nexit 0")
-        test_script.chmod(0o755)
+        test_script.chmod(0o755)  # make sure it's executable. file is already writable only by root due to umask.
     except OSError as e:
         if e.errno == errno.EROFS:
             # ro
@@ -56,3 +58,41 @@ def is_rw_exec_dir(path: str) -> bool:
 
 def escape_filename(filename: str) -> str:
     return filename.replace(":", "-" if is_windows() else ":")
+
+
+def is_owned_by_root(path: Path) -> bool:
+    statbuf = path.stat()
+    return statbuf.st_uid == 0 and statbuf.st_gid == 0
+
+
+def mkdir_owned_root(path: Union[str, Path], mode: int = 0o755) -> None:
+    """
+    Ensures a directory exists and is owned by root.
+
+    If the directory exists and is owned by root, it is left as is.
+    If the directory exists and is not owned by root, it is removed and recreated. If after recreation
+    it is still not owned by root, the function raises.
+    """
+    assert is_root()  # this function behaves as we expect only when run as root
+
+    path = path if isinstance(path, Path) else Path(path)
+    # parent is expected to be root - otherwise, after we create the root-owned directory, it can be removed
+    # as re-created as non-root by a regular user.
+    if not is_owned_by_root(path.parent):
+        raise Exception(f"expected {path.parent} to be owned by root!")
+
+    if path.exists():
+        if is_owned_by_root(path):
+            return
+
+        shutil.rmtree(path)
+
+    try:
+        os.mkdir(path, mode=mode)
+    except FileExistsError:
+        # likely racing with another thread of gprofiler. as long as the directory is root after all, we're good.
+        pass
+
+    if not is_owned_by_root(path):
+        # lost race with someone else?
+        raise Exception(f"Failed to create directory {str(path)} as owned by root")