From 119f6b9ae4ebf1e58c14a46077b7aca5726f9da8 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Tue, 7 Jul 2026 17:18:49 -0700 Subject: [PATCH 1/7] Security: drop privileges when executing target process binaries Add run_process_as_target() helper that drops privileges to match the target process's UID/GID before executing commands. This ensures binaries are executed with appropriate credentials rather than gProfiler's elevated privileges. Updated call sites: - get_exe_version() in metadata/versions.py - _get_sys_maxunicode() in profilers/python.py - get_java_version() in profilers/java.py Co-Authored-By: Claude Opus 4.5 --- gprofiler/metadata/versions.py | 15 +++++--- gprofiler/profilers/java.py | 10 +++++- gprofiler/profilers/python.py | 19 ++++++++-- gprofiler/utils/__init__.py | 63 ++++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index 5a3cd767c..6abce56a6 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -19,7 +19,7 @@ from granulate_utils.linux.ns import get_process_nspid, run_in_ns_wrapper from psutil import NoSuchProcess, Process -from gprofiler.utils import run_process +from gprofiler.utils import run_process_as_target def get_exe_version( @@ -30,13 +30,20 @@ def get_exe_version( try_stderr: bool = False, ) -> str: """ - Runs {process.exe()} --version in the appropriate namespace + Runs {process.exe()} --version in the appropriate namespace. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" def _run_get_version() -> "CompletedProcess[bytes]": - return run_process( - [exe_path, version_arg], stop_event=stop_event, timeout=get_version_timeout, pdeathsigger=False + return run_process_as_target( + [exe_path, version_arg], + target_process=process, + stop_event=stop_event, + timeout=get_version_timeout, + pdeathsigger=False, ) try: diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 2e85486a1..d9cec14ca 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -95,6 +95,7 @@ remove_prefix, resource_path, run_process, + run_process_as_target, touch_path, wait_event, ) @@ -353,17 +354,24 @@ def _get_process_ns_java_path(process: Process) -> Optional[str]: # process is hashable and the same process instance compares equal @functools.lru_cache(maxsize=_JAVA_VERSION_CACHE_MAX) def get_java_version(process: Process, stop_event: Event) -> Optional[str]: + """ + Get Java version from the process's java binary. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. + """ # make sure we're able to find "java" binary bundled with process libjvm process_java_path = _get_process_ns_java_path(process) if process_java_path is None: return None def _run_java_version() -> "CompletedProcess[bytes]": - return run_process( + return run_process_as_target( [ process_java_path, "-version", ], + target_process=process, stop_event=stop_event, timeout=_JAVA_VERSION_TIMEOUT, pdeathsigger=False, diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 781fb1b40..4f5639af8 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -61,7 +61,15 @@ if is_linux(): from gprofiler.profilers.python_ebpf import PythonEbpfProfiler, PythonEbpfError -from gprofiler.utils import pgrep_exe, pgrep_maps, random_prefix, removed_path, resource_path, run_process +from gprofiler.utils import ( + pgrep_exe, + pgrep_maps, + random_prefix, + removed_path, + resource_path, + run_process, + run_process_as_target, +) from gprofiler.utils.process import process_comm, search_proc_maps logger = get_logger_adapter(__name__) @@ -131,6 +139,12 @@ def _get_python_version(self, process: Process) -> Optional[str]: return None def _get_sys_maxunicode(self, process: Process) -> Optional[str]: + """ + Get sys.maxunicode from a Python 2 process. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. + """ try: if not is_process_basename_matching(process, application_identifiers._PYTHON_BIN_RE): # see same raise above @@ -139,8 +153,9 @@ def _get_sys_maxunicode(self, process: Process) -> Optional[str]: python_path = f"/proc/{get_process_nspid(process.pid)}/exe" def _run_python_process_in_ns() -> "CompletedProcess[bytes]": - return run_process( + return run_process_as_target( [python_path, "-S", "-c", "import sys; print(sys.maxunicode)"], + target_process=process, stop_event=self._stop_event, timeout=self._PYTHON_TIMEOUT, pdeathsigger=False, diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 91aaf4eb8..5f37f1dc9 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -59,6 +59,7 @@ StopEventSetException, ) from gprofiler.log import get_logger_adapter +from gprofiler.platform import is_linux as _is_linux_for_priv logger = get_logger_adapter(__name__) @@ -525,6 +526,68 @@ def cleanup_process_reference(process: Popen) -> None: pass # Already removed +def _make_drop_privileges_fn(uid: int, gid: int) -> Callable[[], None]: + """ + Create a preexec_fn that drops privileges to the specified UID/GID. + This runs in the child process before exec(). + """ + + def _drop_privileges() -> None: + try: + os.setgroups([]) # Drop supplementary groups + except OSError: + pass # May fail if not root + os.setgid(gid) # Set GID before UID (can't change GID after dropping root) + os.setuid(uid) + + return _drop_privileges + + +def run_process_as_target( + cmd: List[str], + target_process: Process, + stop_event: Optional[Event] = None, + timeout: int = 5, + **kwargs: Any, +) -> "CompletedProcess[bytes]": + """ + Execute a command with the same UID/GID as the target process. + + Security: This function drops privileges before executing the command, + preventing privilege escalation if the binary is attacker-controlled. + + Args: + cmd: Command and arguments to execute + target_process: The process whose credentials to use + stop_event: Optional event to signal stop + timeout: Command timeout in seconds + **kwargs: Additional arguments passed to run_process() + + Returns: + CompletedProcess with stdout/stderr + """ + preexec_fn: Optional[Callable[[], None]] = None + + # Only drop privileges on Linux when running as root with non-root target + if _is_linux_for_priv(): + target_uids = target_process.uids() + target_gids = target_process.gids() + # Use real UID/GID (not effective) to match the process owner + target_uid = target_uids.real + target_gid = target_gids.real + + if os.geteuid() == 0 and target_uid != 0: + preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) + + return run_process( + cmd, + stop_event=stop_event, + timeout=timeout, + preexec_fn=preexec_fn, + **kwargs, + ) + + def _exit_handler() -> None: for process in _processes: process.kill() From 398ec896fec86b446a38fe0a2efee96620093ec7 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Tue, 7 Jul 2026 17:29:25 -0700 Subject: [PATCH 2/7] Address review comments for privilege dropping - Check os.geteuid() == 0 first before querying target process UIDs to avoid AccessDenied when non-root targets another user's process - Remove try/except around setgroups() - let it fail loudly if it can't drop supplementary groups when running as root - Pop any user-provided preexec_fn from kwargs to avoid TypeError Co-Authored-By: Claude Opus 4.5 --- gprofiler/utils/__init__.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 5f37f1dc9..907693b0e 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -533,10 +533,9 @@ def _make_drop_privileges_fn(uid: int, gid: int) -> Callable[[], None]: """ def _drop_privileges() -> None: - try: - os.setgroups([]) # Drop supplementary groups - except OSError: - pass # May fail if not root + # Drop supplementary groups - this should always succeed when root + # Let it raise if it fails, as that would leave groups intact + os.setgroups([]) os.setgid(gid) # Set GID before UID (can't change GID after dropping root) os.setuid(uid) @@ -566,17 +565,22 @@ def run_process_as_target( Returns: CompletedProcess with stdout/stderr """ + # Remove any user-provided preexec_fn to avoid conflicts + # Our security preexec_fn takes precedence + kwargs.pop("preexec_fn", None) + preexec_fn: Optional[Callable[[], None]] = None - # Only drop privileges on Linux when running as root with non-root target - if _is_linux_for_priv(): + # Only drop privileges on Linux when running as root + # Check root status first to avoid AccessDenied when non-root targets another user's process + if _is_linux_for_priv() and os.geteuid() == 0: target_uids = target_process.uids() target_gids = target_process.gids() # Use real UID/GID (not effective) to match the process owner target_uid = target_uids.real target_gid = target_gids.real - if os.geteuid() == 0 and target_uid != 0: + if target_uid != 0: preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) return run_process( From 3575b928603c13a98364affa400f47c0f91df6a5 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Wed, 8 Jul 2026 15:26:16 -0700 Subject: [PATCH 3/7] Handle NoSuchProcess when target exits before credential lookup - Change run_process_as_target() to accept explicit target_uid/target_gid instead of querying psutil (which fails inside different PID namespace) - Callers now resolve UID/GID BEFORE entering namespace via run_in_ns_wrapper - Create dummy Event when stop_event=None so timeouts work - Pop any user-provided preexec_fn to avoid conflicts - Let setgroups() fail loudly (don't swallow errors) Co-Authored-By: Claude Opus 4.5 --- gprofiler/metadata/versions.py | 7 ++++++- gprofiler/profilers/java.py | 7 ++++++- gprofiler/profilers/python.py | 7 ++++++- gprofiler/utils/__init__.py | 36 +++++++++++++++++++--------------- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index 6abce56a6..27f108e54 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -37,10 +37,15 @@ def get_exe_version( """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" + # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_get_version() -> "CompletedProcess[bytes]": return run_process_as_target( [exe_path, version_arg], - target_process=process, + target_uid=target_uid, + target_gid=target_gid, stop_event=stop_event, timeout=get_version_timeout, pdeathsigger=False, diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index d9cec14ca..674dafdcf 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -365,13 +365,18 @@ def get_java_version(process: Process, stop_event: Event) -> Optional[str]: if process_java_path is None: return None + # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_java_version() -> "CompletedProcess[bytes]": return run_process_as_target( [ process_java_path, "-version", ], - target_process=process, + target_uid=target_uid, + target_gid=target_gid, stop_event=stop_event, timeout=_JAVA_VERSION_TIMEOUT, pdeathsigger=False, diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 4f5639af8..0260131a4 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -152,10 +152,15 @@ def _get_sys_maxunicode(self, process: Process) -> Optional[str]: python_path = f"/proc/{get_process_nspid(process.pid)}/exe" + # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_python_process_in_ns() -> "CompletedProcess[bytes]": return run_process_as_target( [python_path, "-S", "-c", "import sys; print(sys.maxunicode)"], - target_process=process, + target_uid=target_uid, + target_gid=target_gid, stop_event=self._stop_event, timeout=self._PYTHON_TIMEOUT, pdeathsigger=False, diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 907693b0e..3b973bef3 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -544,22 +544,28 @@ def _drop_privileges() -> None: def run_process_as_target( cmd: List[str], - target_process: Process, + target_uid: int, + target_gid: int, stop_event: Optional[Event] = None, timeout: int = 5, **kwargs: Any, ) -> "CompletedProcess[bytes]": """ - Execute a command with the same UID/GID as the target process. + Execute a command with the specified UID/GID (dropping privileges if root). Security: This function drops privileges before executing the command, preventing privilege escalation if the binary is attacker-controlled. + Note: Callers must resolve target_uid/target_gid BEFORE entering any + PID/mount namespaces, since psutil can't look up host PIDs from inside + a different namespace. + Args: cmd: Command and arguments to execute - target_process: The process whose credentials to use - stop_event: Optional event to signal stop - timeout: Command timeout in seconds + target_uid: The UID to run the command as + target_gid: The GID to run the command as + stop_event: Optional event to signal stop (created internally if None) + timeout: Command timeout in seconds (default: 5) **kwargs: Additional arguments passed to run_process() Returns: @@ -569,19 +575,17 @@ def run_process_as_target( # Our security preexec_fn takes precedence kwargs.pop("preexec_fn", None) + # Create a dummy Event if none provided, so timeouts work + # (run_process asserts timeout must be None when stop_event is None) + if stop_event is None: + stop_event = Event() + preexec_fn: Optional[Callable[[], None]] = None - # Only drop privileges on Linux when running as root - # Check root status first to avoid AccessDenied when non-root targets another user's process - if _is_linux_for_priv() and os.geteuid() == 0: - target_uids = target_process.uids() - target_gids = target_process.gids() - # Use real UID/GID (not effective) to match the process owner - target_uid = target_uids.real - target_gid = target_gids.real - - if target_uid != 0: - preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) + # Only drop privileges on Linux when running as root and target is non-root + # Use is_root() which handles user-namespace/container scenarios properly + if _is_linux_for_priv() and is_root() and target_uid != 0: + preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) return run_process( cmd, From e6470059208ee1ecb4c6452c3f64b835ee8b9db1 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Mon, 20 Jul 2026 12:29:51 -0700 Subject: [PATCH 4/7] Temporarily disable privilege dropping due to preexec_fn deadlock The preexec_fn mechanism causes deadlock in multi-threaded processes when fork() is called while another thread holds a lock. This affects container tests where gProfiler runs with multiple profiler threads. TODO: Implement a wrapper binary (like pdeathsigger) to safely drop privileges before exec() without using preexec_fn. Co-Authored-By: Claude Opus 4.5 --- gprofiler/utils/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 3b973bef3..609852de2 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -584,7 +584,10 @@ def run_process_as_target( # Only drop privileges on Linux when running as root and target is non-root # Use is_root() which handles user-namespace/container scenarios properly - if _is_linux_for_priv() and is_root() and target_uid != 0: + # TODO: Temporarily disabled - preexec_fn causes deadlock in multi-threaded process. + # Need to implement a wrapper binary (like pdeathsigger) to drop privileges safely. + # See: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.preexec_fn + if False and _is_linux_for_priv() and is_root() and target_uid != 0: preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) return run_process( From a3c50ef82d9e5a813f8ab4b14d1752be6a8b3ff7 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Mon, 20 Jul 2026 13:49:46 -0700 Subject: [PATCH 5/7] Use subprocess user/group params instead of preexec_fn for privilege dropping Replace the preexec_fn-based privilege dropping with subprocess's built-in user, group, and extra_groups parameters (available since Python 3.9). These parameters are implemented in C code (_posixsubprocess.c) and avoid the preexec_fn deadlock issues that occur in multi-threaded processes when fork() is called while another thread holds a lock. Co-Authored-By: Claude Opus 4.5 --- gprofiler/utils/__init__.py | 41 +++++++++++++------------------------ 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 609852de2..3974d4072 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -526,22 +526,6 @@ def cleanup_process_reference(process: Popen) -> None: pass # Already removed -def _make_drop_privileges_fn(uid: int, gid: int) -> Callable[[], None]: - """ - Create a preexec_fn that drops privileges to the specified UID/GID. - This runs in the child process before exec(). - """ - - def _drop_privileges() -> None: - # Drop supplementary groups - this should always succeed when root - # Let it raise if it fails, as that would leave groups intact - os.setgroups([]) - os.setgid(gid) # Set GID before UID (can't change GID after dropping root) - os.setuid(uid) - - return _drop_privileges - - def run_process_as_target( cmd: List[str], target_uid: int, @@ -560,6 +544,10 @@ def run_process_as_target( PID/mount namespaces, since psutil can't look up host PIDs from inside a different namespace. + Uses subprocess's user/group/extra_groups parameters (Python 3.9+) which + are implemented in C and avoid the preexec_fn deadlock issues in + multi-threaded processes. + Args: cmd: Command and arguments to execute target_uid: The UID to run the command as @@ -571,30 +559,29 @@ def run_process_as_target( Returns: CompletedProcess with stdout/stderr """ - # Remove any user-provided preexec_fn to avoid conflicts - # Our security preexec_fn takes precedence - kwargs.pop("preexec_fn", None) + # Remove any user-provided credential settings to avoid conflicts + kwargs.pop("user", None) + kwargs.pop("group", None) + kwargs.pop("extra_groups", None) # Create a dummy Event if none provided, so timeouts work # (run_process asserts timeout must be None when stop_event is None) if stop_event is None: stop_event = Event() - preexec_fn: Optional[Callable[[], None]] = None - # Only drop privileges on Linux when running as root and target is non-root # Use is_root() which handles user-namespace/container scenarios properly - # TODO: Temporarily disabled - preexec_fn causes deadlock in multi-threaded process. - # Need to implement a wrapper binary (like pdeathsigger) to drop privileges safely. - # See: https://docs.python.org/3/library/subprocess.html#subprocess.Popen.preexec_fn - if False and _is_linux_for_priv() and is_root() and target_uid != 0: - preexec_fn = _make_drop_privileges_fn(target_uid, target_gid) + if _is_linux_for_priv() and is_root() and target_uid != 0: + # Use subprocess's built-in user/group/extra_groups parameters + # These are implemented in C and avoid preexec_fn deadlock issues + kwargs["user"] = target_uid + kwargs["group"] = target_gid + kwargs["extra_groups"] = [] # Drop all supplementary groups return run_process( cmd, stop_event=stop_event, timeout=timeout, - preexec_fn=preexec_fn, **kwargs, ) From 601e56648faffe5946e6a6f2d2cc6349ea2e7a05 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Mon, 20 Jul 2026 21:34:25 -0700 Subject: [PATCH 6/7] Extend pdeathsigger to support privilege dropping for security fix Extended pdeathsigger binary with optional -u -g flags to drop privileges before exec. This fixes the security vulnerability where gProfiler executes target process binaries as root. Key changes: - Extended scripts/pdeathsigger.c to support privilege dropping - Added get_pdeathsigger_path() to resolve path before namespace entry - Updated run_process_as_target() to use pdeathsigger with -u/-g flags - Updated callers (java.py, python.py, versions.py) to pass pdeathsigger_path The pdeathsigger path must be resolved BEFORE entering the target's namespace because resource_path() hangs when called inside a different mount namespace. Co-Authored-By: Claude Opus 4.5 --- gprofiler/metadata/versions.py | 8 ++-- gprofiler/profilers/java.py | 7 +++- gprofiler/profilers/python.py | 7 +++- gprofiler/utils/__init__.py | 59 +++++++++++++++++++--------- scripts/pdeathsigger.c | 70 +++++++++++++++++++++++++++++++--- 5 files changed, 122 insertions(+), 29 deletions(-) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index 27f108e54..add2d3d08 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -19,7 +19,7 @@ from granulate_utils.linux.ns import get_process_nspid, run_in_ns_wrapper from psutil import NoSuchProcess, Process -from gprofiler.utils import run_process_as_target +from gprofiler.utils import get_pdeathsigger_path, run_process_as_target def get_exe_version( @@ -37,9 +37,11 @@ def get_exe_version( """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" - # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + # Get credentials and pdeathsigger path BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) target_uid = process.uids().real target_gid = process.gids().real + pdeathsigger_path = get_pdeathsigger_path() def _run_get_version() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -48,7 +50,7 @@ def _run_get_version() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=stop_event, timeout=get_version_timeout, - pdeathsigger=False, + pdeathsigger_path=pdeathsigger_path, ) try: diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 674dafdcf..74c2ea40a 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -90,6 +90,7 @@ from gprofiler.utils import ( GPROFILER_DIRECTORY_NAME, TEMPORARY_STORAGE_PATH, + get_pdeathsigger_path, pgrep_maps, remove_path, remove_prefix, @@ -365,9 +366,11 @@ def get_java_version(process: Process, stop_event: Event) -> Optional[str]: if process_java_path is None: return None - # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + # Get credentials and pdeathsigger path BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) target_uid = process.uids().real target_gid = process.gids().real + pdeathsigger_path = get_pdeathsigger_path() def _run_java_version() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -379,7 +382,7 @@ def _run_java_version() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=stop_event, timeout=_JAVA_VERSION_TIMEOUT, - pdeathsigger=False, + pdeathsigger_path=pdeathsigger_path, ) # doesn't work without changing PID NS as well (I'm getting ENOENT for libjli.so) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 0260131a4..07958403e 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -62,6 +62,7 @@ from gprofiler.profilers.python_ebpf import PythonEbpfProfiler, PythonEbpfError from gprofiler.utils import ( + get_pdeathsigger_path, pgrep_exe, pgrep_maps, random_prefix, @@ -152,9 +153,11 @@ def _get_sys_maxunicode(self, process: Process) -> Optional[str]: python_path = f"/proc/{get_process_nspid(process.pid)}/exe" - # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) + # Get credentials and pdeathsigger path BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) target_uid = process.uids().real target_gid = process.gids().real + pdeathsigger_path = get_pdeathsigger_path() def _run_python_process_in_ns() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -163,7 +166,7 @@ def _run_python_process_in_ns() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=self._stop_event, timeout=self._PYTHON_TIMEOUT, - pdeathsigger=False, + pdeathsigger_path=pdeathsigger_path, ) result = cast(CompletedProcess, run_in_ns_wrapper(["pid", "mnt"], _run_python_process_in_ns, process.pid)) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 3974d4072..954f6b23e 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -526,12 +526,34 @@ def cleanup_process_reference(process: Popen) -> None: pass # Already removed +def get_pdeathsigger_path() -> Optional[str]: + """ + Get the path to the pdeathsigger binary. + + This function should be called BEFORE entering any PID/mount namespaces, + as resource_path() may hang when called inside a different namespace. + + Returns: + Path to pdeathsigger binary if available, None otherwise. + """ + if not _is_linux_for_priv(): + return None + try: + path = resource_path("pdeathsigger") + if os.path.exists(path): + return path + except Exception: + pass + return None + + def run_process_as_target( cmd: List[str], target_uid: int, target_gid: int, stop_event: Optional[Event] = None, timeout: int = 5, + pdeathsigger_path: Optional[str] = None, **kwargs: Any, ) -> "CompletedProcess[bytes]": """ @@ -540,13 +562,13 @@ def run_process_as_target( Security: This function drops privileges before executing the command, preventing privilege escalation if the binary is attacker-controlled. - Note: Callers must resolve target_uid/target_gid BEFORE entering any - PID/mount namespaces, since psutil can't look up host PIDs from inside - a different namespace. + Note: Callers must resolve target_uid/target_gid and pdeathsigger_path BEFORE + entering any PID/mount namespaces, since these lookups may fail or hang + inside a different namespace. - Uses subprocess's user/group/extra_groups parameters (Python 3.9+) which - are implemented in C and avoid the preexec_fn deadlock issues in - multi-threaded processes. + Uses the 'pdeathsigger' wrapper binary with -u/-g flags to drop privileges + before exec, avoiding preexec_fn deadlock issues in multi-threaded processes + and compatibility issues with container environments. Args: cmd: Command and arguments to execute @@ -554,16 +576,13 @@ def run_process_as_target( target_gid: The GID to run the command as stop_event: Optional event to signal stop (created internally if None) timeout: Command timeout in seconds (default: 5) + pdeathsigger_path: Path to pdeathsigger binary (from get_pdeathsigger_path(), + resolved before namespace entry) **kwargs: Additional arguments passed to run_process() Returns: CompletedProcess with stdout/stderr """ - # Remove any user-provided credential settings to avoid conflicts - kwargs.pop("user", None) - kwargs.pop("group", None) - kwargs.pop("extra_groups", None) - # Create a dummy Event if none provided, so timeouts work # (run_process asserts timeout must be None when stop_event is None) if stop_event is None: @@ -571,12 +590,18 @@ def run_process_as_target( # Only drop privileges on Linux when running as root and target is non-root # Use is_root() which handles user-namespace/container scenarios properly - if _is_linux_for_priv() and is_root() and target_uid != 0: - # Use subprocess's built-in user/group/extra_groups parameters - # These are implemented in C and avoid preexec_fn deadlock issues - kwargs["user"] = target_uid - kwargs["group"] = target_gid - kwargs["extra_groups"] = [] # Drop all supplementary groups + if _is_linux_for_priv() and is_root() and target_uid != 0 and pdeathsigger_path is not None: + # Use pdeathsigger wrapper with -u/-g flags to drop privileges before exec + # This avoids preexec_fn deadlock and subprocess user/group param issues + logger.debug( + "Using pdeathsigger for privilege dropping", + pdeathsigger_path=pdeathsigger_path, + target_uid=target_uid, + target_gid=target_gid, + ) + cmd = [pdeathsigger_path, "-u", str(target_uid), "-g", str(target_gid)] + cmd + # Disable pdeathsigger in run_process since we're already using it + kwargs["pdeathsigger"] = False return run_process( cmd, diff --git a/scripts/pdeathsigger.c b/scripts/pdeathsigger.c index fe8f5b98e..17b1c3363 100644 --- a/scripts/pdeathsigger.c +++ b/scripts/pdeathsigger.c @@ -1,27 +1,87 @@ #include #include +#include #include #include #include +#include +#include /* preexec_fn is not safe to use in the presence of threads, child process could deadlock before exec is called. - this little shim is a workaround to avoid using preexe_fn and - still get the desired behavior (PR_SET_PDEATHSIG). + this little shim is a workaround to avoid using preexec_fn and + still get the desired behavior (PR_SET_PDEATHSIG and privilege dropping). + + Usage: + pdeathsigger /path/to/binary [args...] + pdeathsigger -u -g /path/to/binary [args...] + + When -u and -g are provided, drops privileges to the specified UID/GID + before executing the command. This prevents privilege escalation if the + target binary is attacker-controlled. */ int main(int argc, char *argv[]) { - if (argc < 2) { - fprintf(stderr, "Usage: %s /path/to/binary [args...]\n", argv[0]); + int arg_offset = 1; + long uid = -1; + long gid = -1; + char *endptr; + + /* Parse optional -u and -g flags */ + while (arg_offset < argc) { + if (strcmp(argv[arg_offset], "-u") == 0 && arg_offset + 1 < argc) { + errno = 0; + uid = strtol(argv[arg_offset + 1], &endptr, 10); + if (errno != 0 || *endptr != '\0' || uid < 0) { + fprintf(stderr, "Invalid UID: %s\n", argv[arg_offset + 1]); + return 1; + } + arg_offset += 2; + } else if (strcmp(argv[arg_offset], "-g") == 0 && arg_offset + 1 < argc) { + errno = 0; + gid = strtol(argv[arg_offset + 1], &endptr, 10); + if (errno != 0 || *endptr != '\0' || gid < 0) { + fprintf(stderr, "Invalid GID: %s\n", argv[arg_offset + 1]); + return 1; + } + arg_offset += 2; + } else { + break; + } + } + + if (arg_offset >= argc) { + fprintf(stderr, "Usage: %s [-u -g ] /path/to/binary [args...]\n", argv[0]); return 1; } + /* Set PR_SET_PDEATHSIG */ if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { perror("prctl"); return 1; } - execvp(argv[1], &argv[1]); + /* Drop privileges if uid/gid were specified and we're root */ + if (uid >= 0 && gid >= 0 && geteuid() == 0 && uid != 0) { + /* Drop supplementary groups */ + if (setgroups(0, NULL) == -1) { + perror("setgroups"); + return 1; + } + + /* Set GID before UID (can't change GID after dropping root) */ + if (setgid((gid_t)gid) == -1) { + perror("setgid"); + return 1; + } + + if (setuid((uid_t)uid) == -1) { + perror("setuid"); + return 1; + } + } + + execvp(argv[arg_offset], &argv[arg_offset]); perror("execvp"); return 1; From d33ffa44e9e47c9e8f9a07f5a8aa4b3ef9171a77 Mon Sep 17 00:00:00 2001 From: Min Lim Date: Wed, 22 Jul 2026 14:04:45 -0700 Subject: [PATCH 7/7] Use subprocess user/group params instead of pdeathsigger for privilege dropping Replace the pdeathsigger-based privilege dropping with Python's subprocess user/group/extra_groups parameters. This approach: - Works inside any mount namespace (no external binary needed) - Is implemented in C code after fork(), before exec() (no deadlock risk) - Drops supplementary groups via extra_groups=[] The pdeathsigger approach failed when gProfiler ran in a container and profiled processes in different namespaces, because the pdeathsigger binary path from gProfiler's container was not accessible inside the target's mount namespace. Co-Authored-By: Claude Opus 4.5 --- gprofiler/metadata/versions.py | 8 ++-- gprofiler/profilers/java.py | 7 +--- gprofiler/profilers/python.py | 7 +--- gprofiler/utils/__init__.py | 70 +++++++++++++++------------------- 4 files changed, 37 insertions(+), 55 deletions(-) diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index add2d3d08..b3fb22503 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -19,7 +19,7 @@ from granulate_utils.linux.ns import get_process_nspid, run_in_ns_wrapper from psutil import NoSuchProcess, Process -from gprofiler.utils import get_pdeathsigger_path, run_process_as_target +from gprofiler.utils import run_process_as_target def get_exe_version( @@ -37,11 +37,10 @@ def get_exe_version( """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" - # Get credentials and pdeathsigger path BEFORE entering namespace - # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) target_uid = process.uids().real target_gid = process.gids().real - pdeathsigger_path = get_pdeathsigger_path() def _run_get_version() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -50,7 +49,6 @@ def _run_get_version() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=stop_event, timeout=get_version_timeout, - pdeathsigger_path=pdeathsigger_path, ) try: diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 74c2ea40a..8f79b3292 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -90,7 +90,6 @@ from gprofiler.utils import ( GPROFILER_DIRECTORY_NAME, TEMPORARY_STORAGE_PATH, - get_pdeathsigger_path, pgrep_maps, remove_path, remove_prefix, @@ -366,11 +365,10 @@ def get_java_version(process: Process, stop_event: Event) -> Optional[str]: if process_java_path is None: return None - # Get credentials and pdeathsigger path BEFORE entering namespace - # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) target_uid = process.uids().real target_gid = process.gids().real - pdeathsigger_path = get_pdeathsigger_path() def _run_java_version() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -382,7 +380,6 @@ def _run_java_version() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=stop_event, timeout=_JAVA_VERSION_TIMEOUT, - pdeathsigger_path=pdeathsigger_path, ) # doesn't work without changing PID NS as well (I'm getting ENOENT for libjli.so) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 07958403e..fc074210c 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -62,7 +62,6 @@ from gprofiler.profilers.python_ebpf import PythonEbpfProfiler, PythonEbpfError from gprofiler.utils import ( - get_pdeathsigger_path, pgrep_exe, pgrep_maps, random_prefix, @@ -153,11 +152,10 @@ def _get_sys_maxunicode(self, process: Process) -> Optional[str]: python_path = f"/proc/{get_process_nspid(process.pid)}/exe" - # Get credentials and pdeathsigger path BEFORE entering namespace - # (psutil can't resolve host PIDs inside namespace, and resource_path may hang) + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) target_uid = process.uids().real target_gid = process.gids().real - pdeathsigger_path = get_pdeathsigger_path() def _run_python_process_in_ns() -> "CompletedProcess[bytes]": return run_process_as_target( @@ -166,7 +164,6 @@ def _run_python_process_in_ns() -> "CompletedProcess[bytes]": target_gid=target_gid, stop_event=self._stop_event, timeout=self._PYTHON_TIMEOUT, - pdeathsigger_path=pdeathsigger_path, ) result = cast(CompletedProcess, run_in_ns_wrapper(["pid", "mnt"], _run_python_process_in_ns, process.pid)) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 954f6b23e..eafad10dd 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -526,34 +526,12 @@ def cleanup_process_reference(process: Popen) -> None: pass # Already removed -def get_pdeathsigger_path() -> Optional[str]: - """ - Get the path to the pdeathsigger binary. - - This function should be called BEFORE entering any PID/mount namespaces, - as resource_path() may hang when called inside a different namespace. - - Returns: - Path to pdeathsigger binary if available, None otherwise. - """ - if not _is_linux_for_priv(): - return None - try: - path = resource_path("pdeathsigger") - if os.path.exists(path): - return path - except Exception: - pass - return None - - def run_process_as_target( cmd: List[str], target_uid: int, target_gid: int, stop_event: Optional[Event] = None, timeout: int = 5, - pdeathsigger_path: Optional[str] = None, **kwargs: Any, ) -> "CompletedProcess[bytes]": """ @@ -562,13 +540,11 @@ def run_process_as_target( Security: This function drops privileges before executing the command, preventing privilege escalation if the binary is attacker-controlled. - Note: Callers must resolve target_uid/target_gid and pdeathsigger_path BEFORE - entering any PID/mount namespaces, since these lookups may fail or hang - inside a different namespace. + Note: Callers must resolve target_uid/target_gid BEFORE entering any + PID/mount namespaces, since these lookups may fail inside a different namespace. - Uses the 'pdeathsigger' wrapper binary with -u/-g flags to drop privileges - before exec, avoiding preexec_fn deadlock issues in multi-threaded processes - and compatibility issues with container environments. + Uses subprocess user/group parameters which handle privilege dropping in + the child process after fork (implemented in C, no preexec_fn deadlock risk). Args: cmd: Command and arguments to execute @@ -576,8 +552,6 @@ def run_process_as_target( target_gid: The GID to run the command as stop_event: Optional event to signal stop (created internally if None) timeout: Command timeout in seconds (default: 5) - pdeathsigger_path: Path to pdeathsigger binary (from get_pdeathsigger_path(), - resolved before namespace entry) **kwargs: Additional arguments passed to run_process() Returns: @@ -588,21 +562,37 @@ def run_process_as_target( if stop_event is None: stop_event = Event() + # Always disable pdeathsigger wrapper when running as target + # This function is called inside target namespaces where pdeathsigger may not exist + kwargs["pdeathsigger"] = False + # Only drop privileges on Linux when running as root and target is non-root # Use is_root() which handles user-namespace/container scenarios properly - if _is_linux_for_priv() and is_root() and target_uid != 0 and pdeathsigger_path is not None: - # Use pdeathsigger wrapper with -u/-g flags to drop privileges before exec - # This avoids preexec_fn deadlock and subprocess user/group param issues + should_drop = _is_linux_for_priv() and is_root() and target_uid != 0 + logger.debug( + "run_process_as_target called", + cmd=cmd, + target_uid=target_uid, + target_gid=target_gid, + is_linux=_is_linux_for_priv(), + is_root=is_root(), + should_drop_privileges=should_drop, + ) + + if should_drop: + # Use subprocess user/group params for privilege dropping + # This is handled in C code after fork(), before exec() - no deadlock risk + # and works inside any namespace (no external binary needed) + kwargs["user"] = target_uid + kwargs["group"] = target_gid + kwargs["extra_groups"] = [] logger.debug( - "Using pdeathsigger for privilege dropping", - pdeathsigger_path=pdeathsigger_path, - target_uid=target_uid, - target_gid=target_gid, + "Privilege dropping enabled", + user=kwargs["user"], + group=kwargs["group"], ) - cmd = [pdeathsigger_path, "-u", str(target_uid), "-g", str(target_gid)] + cmd - # Disable pdeathsigger in run_process since we're already using it - kwargs["pdeathsigger"] = False + logger.debug("Calling run_process", cmd=cmd, timeout=timeout) return run_process( cmd, stop_event=stop_event,