Security: drop privileges when executing target process binaries#1048
Closed
mlim19 wants to merge 7 commits into
Closed
Security: drop privileges when executing target process binaries#1048mlim19 wants to merge 7 commits into
mlim19 wants to merge 7 commits into
Conversation
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a safer way to execute “version probing” commands against target process binaries by dropping privileges to the target process’s UID/GID, reducing the impact of attacker-controlled binaries when gProfiler runs as root.
Changes:
- Added
run_process_as_target()helper (with a privilege-droppingpreexec_fn) ingprofiler/utils/__init__.py. - Updated executable version detection to run under the target process’s credentials (
get_exe_version(), Python maxunicode probe, Java version probe).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
gprofiler/utils/__init__.py |
Adds privilege-dropping helper used to execute commands as the target process owner. |
gprofiler/metadata/versions.py |
Uses run_process_as_target() for {exe} --version probing. |
gprofiler/profilers/python.py |
Uses run_process_as_target() when probing sys.maxunicode. |
gprofiler/profilers/java.py |
Uses run_process_as_target() for java -version probing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
gprofiler/profilers/java.py:378
- Because this is executed inside run_in_ns_wrapper(["pid","mnt"], ...), run_process_as_target may be unable to read target_process.uids()/gids() after the PID namespace switch (host PID might not exist in the entered namespace). If that happens, the helper can skip privilege dropping and execute java as root, undermining the security goal.
Resolve uid/gid before entering the namespaces and pass them into the namespace-executed callable (or extend run_process_as_target to accept explicit uid/gid), and avoid a “run as root” fallback when privilege dropping cannot be established.
target_process=process,
stop_event=stop_event,
timeout=_JAVA_VERSION_TIMEOUT,
pdeathsigger=False,
)
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 8, 2026 22:26
879d264 to
1b359be
Compare
Comment on lines
+529
to
+533
| 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(). | ||
| """ |
- 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 <noreply@anthropic.com>
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 8, 2026 22:50
1b359be to
3575b92
Compare
Comment on lines
+574
to
+596
| # Remove any user-provided preexec_fn to avoid conflicts | ||
| # 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 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, | ||
| stop_event=stop_event, | ||
| timeout=timeout, | ||
| preexec_fn=preexec_fn, | ||
| **kwargs, | ||
| ) |
Comment on lines
+545
to
+552
| def run_process_as_target( | ||
| cmd: List[str], | ||
| target_uid: int, | ||
| target_gid: int, | ||
| stop_event: Optional[Event] = None, | ||
| timeout: int = 5, | ||
| **kwargs: Any, | ||
| ) -> "CompletedProcess[bytes]": |
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 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
gprofiler/profilers/python.py:146
- This docstring claims the command is executed with the target process's UID/GID, but
run_process_as_target()currently does not drop privileges (see gprofiler/utils/init.py). Please update the docstring to avoid asserting a security property that isn't implemented yet.
Security: Executes with the target process's UID/GID to prevent
privilege escalation if the binary is attacker-controlled.
gprofiler/metadata/versions.py:36
- This docstring claims the command is executed with the target process's UID/GID, but
run_process_as_target()currently does not drop privileges (see gprofiler/utils/init.py). Please update the docstring to avoid asserting a security property that isn't implemented yet.
Security: Executes with the target process's UID/GID to prevent
privilege escalation if the binary is attacker-controlled.
Comment on lines
+554
to
+558
| 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. | ||
|
|
Comment on lines
+587
to
+591
| # 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) |
Comment on lines
+360
to
+361
| Security: Executes with the target process's UID/GID to prevent | ||
| privilege escalation if the binary is attacker-controlled. |
Comment on lines
+567
to
+570
| # 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() |
Comment on lines
+574
to
+579
| 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 |
Comment on lines
+40
to
+42
| # Get credentials BEFORE entering namespace (psutil can't resolve host PIDs inside namespace) | ||
| target_uid = process.uids().real | ||
| target_gid = process.gids().real |
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 21, 2026 00:22
6d47987 to
3153b7a
Compare
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 21, 2026 22:01
ad601c8 to
e84bff0
Compare
Comment on lines
+567
to
+575
| # Only drop privileges on Linux when running as root and target is non-root | ||
| # Use is_root() which handles user-namespace/container scenarios properly | ||
| # NOTE: Privilege dropping is currently disabled because run_process_as_target | ||
| # is called inside run_in_ns_wrapper (target's namespace), where we can't | ||
| # access gProfiler's resources (privdrop binary). The resource_path() call | ||
| # hangs when trying to extract resources inside a different mount namespace. | ||
| # TODO: Resolve privdrop_path BEFORE entering namespace and pass it here. | ||
| if False and _is_linux_for_priv() and is_root() and target_uid != 0: | ||
| # Try to use privdrop wrapper binary if available |
Comment on lines
+529
to
+536
| def run_process_as_target( | ||
| cmd: List[str], | ||
| target_uid: int, | ||
| target_gid: int, | ||
| stop_event: Optional[Event] = None, | ||
| timeout: int = 5, | ||
| **kwargs: Any, | ||
| ) -> "CompletedProcess[bytes]": |
Comment on lines
+562
to
+565
| # 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() |
Comment on lines
+42
to
+46
| long uid = strtol(argv[1], &endptr, 10); | ||
| if (errno != 0 || *endptr != '\0' || uid < 0) { | ||
| fprintf(stderr, "Invalid UID: %s\n", argv[1]); | ||
| return 1; | ||
| } |
Comment on lines
+50
to
+54
| long gid = strtol(argv[2], &endptr, 10); | ||
| if (errno != 0 || *endptr != '\0' || gid < 0) { | ||
| fprintf(stderr, "Invalid GID: %s\n", argv[2]); | ||
| return 1; | ||
| } |
Comment on lines
+65
to
+73
| if (setgid((gid_t)gid) == -1) { | ||
| perror("setgid"); | ||
| return 1; | ||
| } | ||
|
|
||
| if (setuid((uid_t)uid) == -1) { | ||
| perror("setuid"); | ||
| return 1; | ||
| } |
Comment on lines
+56
to
+62
| /* Only drop privileges if we're root and target is non-root */ | ||
| if (geteuid() == 0 && uid != 0) { | ||
| /* Drop supplementary groups */ | ||
| if (setgroups(0, NULL) == -1) { | ||
| perror("setgroups"); | ||
| return 1; | ||
| } |
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
2 times, most recently
from
July 22, 2026 05:26
3955091 to
3556532
Compare
Comment on lines
+34
to
+38
| 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; | ||
| } |
Comment on lines
+42
to
+46
| 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; | ||
| } |
Comment on lines
+73
to
+81
| if (setgid((gid_t)gid) == -1) { | ||
| perror("setgid"); | ||
| return 1; | ||
| } | ||
|
|
||
| if (setuid((uid_t)uid) == -1) { | ||
| perror("setuid"); | ||
| return 1; | ||
| } |
Comment on lines
+31
to
+51
| 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; | ||
| } | ||
| } |
Comment on lines
+64
to
+65
| /* Drop privileges if uid/gid were specified and we're root */ | ||
| if (uid >= 0 && gid >= 0 && geteuid() == 0 && uid != 0) { |
Comment on lines
+586
to
+589
| # 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() |
Comment on lines
+541
to
+547
| try: | ||
| path = resource_path("pdeathsigger") | ||
| if os.path.exists(path): | ||
| return path | ||
| except Exception: | ||
| pass | ||
| return None |
| FROM centos${AP_BUILDER_CENTOS} AS async-profiler-builder-glibc | ||
| WORKDIR /tmp | ||
| COPY scripts/async_profiler_env_glibc.sh scripts/fix_centos7.sh scripts/pdeathsigger.c ./ | ||
| COPY scripts/async_profiler_env_glibc.sh scripts/fix_centos7.sh scripts/pdeathsigger.c scripts/privdrop.c ./ |
Comment on lines
+86
to
+87
| gcc -static -o pdeathsigger pdeathsigger.c && \ | ||
| gcc -static -o privdrop privdrop.c |
Comment on lines
+257
to
+258
| COPY --from=async-profiler-builder-glibc /tmp/pdeathsigger gprofiler/resources/pdeathsigger | ||
| COPY --from=async-profiler-builder-glibc /tmp/privdrop gprofiler/resources/privdrop |
Comment on lines
+30
to
+51
| /* 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; | ||
| } | ||
| } |
Comment on lines
+64
to
+65
| /* Drop privileges if uid/gid were specified and we're root */ | ||
| if (uid >= 0 && gid >= 0 && geteuid() == 0 && uid != 0) { |
| 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); |
| 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); |
| } | ||
|
|
||
| /* Set GID before UID (can't change GID after dropping root) */ | ||
| if (setgid((gid_t)gid) == -1) { |
Comment on lines
+591
to
+611
| # 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 | ||
| 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, | ||
| stop_event=stop_event, | ||
| timeout=timeout, | ||
| **kwargs, | ||
| ) |
Comment on lines
+42
to
+43
| target_uid = process.uids().real | ||
| target_gid = process.gids().real |
| FROM centos${AP_BUILDER_CENTOS} AS async-profiler-builder-glibc | ||
| WORKDIR /tmp | ||
| COPY scripts/async_profiler_env_glibc.sh scripts/fix_centos7.sh scripts/pdeathsigger.c ./ | ||
| COPY scripts/async_profiler_env_glibc.sh scripts/fix_centos7.sh scripts/pdeathsigger.c scripts/privdrop.c ./ |
Comment on lines
+86
to
+87
| gcc -static -o pdeathsigger pdeathsigger.c && \ | ||
| gcc -static -o privdrop privdrop.c |
Comment on lines
+257
to
+258
| COPY --from=async-profiler-builder-glibc /tmp/pdeathsigger gprofiler/resources/pdeathsigger | ||
| COPY --from=async-profiler-builder-glibc /tmp/privdrop gprofiler/resources/privdrop |
Extended pdeathsigger binary with optional -u <uid> -g <gid> 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 <noreply@anthropic.com>
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 22, 2026 18:18
3556532 to
601e566
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
gprofiler/utils/init.py:593
- run_process_as_target() can (1) implicitly re-enable the default pdeathsigger wrapping (start_process prepends it when pdeathsigger isn't explicitly disabled), which contradicts the intent to resolve pdeathsigger_path before namespace entry, and (2) fall back to executing the target command as root when targeting a non-root UID if pdeathsigger_path is missing, undermining the privilege-dropping guarantee. Consider explicitly disabling implicit wrapping and failing closed when privilege dropping is required but the wrapper path isn't available.
# 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()
# 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:
scripts/pdeathsigger.c:56
- The new -u/-g parsing allows providing only one of the flags; in that case uid/gid stays partially unset and the program silently proceeds without dropping privileges. That makes misconfiguration easy and can lead to unexpectedly executing the target binary as root.
if (arg_offset >= argc) {
fprintf(stderr, "Usage: %s [-u <uid> -g <gid>] /path/to/binary [args...]\n", argv[0]);
return 1;
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/pdeathsigger.c:56
- If only one of -u or -g is provided, the program silently proceeds without dropping privileges because the drop branch requires both uid and gid. This is a security footgun (callers may assume privileges were dropped). Treat partial specification as a usage error.
/* 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 <uid> -g <gid>] /path/to/binary [args...]\n", argv[0]);
return 1;
}
gprofiler/utils/init.py:570
- run_process_as_target() only sets pdeathsigger=False when it is also dropping privileges. When called from run_in_ns_wrapper() in this PR, non-root executions will still default to pdeathsigger=True, which prepends the bundled pdeathsigger binary (likely not present/visible inside the target mount namespace) and can break version detection. Default pdeathsigger to False unless explicitly overridden by the caller.
# 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()
# 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 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)
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 23, 2026 00:21
8e74178 to
2e9d025
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
scripts/pdeathsigger.c:66
- The new -u/-g parsing allows specifying only one of the flags; in that case the program silently executes the target command without dropping privileges. Also, the privilege-drop condition skips when uid==0, which contradicts the documented behavior of dropping to the provided UID/GID when both flags are set (e.g., "-u 0 -g 1000" won't apply the GID). Add a validation requiring both flags together and drop the uid!=0 guard.
/* Drop privileges if uid/gid were specified and we're root */
if (uid >= 0 && gid >= 0 && geteuid() == 0 && uid != 0) {
/* Drop supplementary groups */
gprofiler/utils/init.py:567
- run_process_as_target() advertises that **kwargs are forwarded to run_process(), but it unconditionally overwrites the caller's pdeathsigger setting. Either document that pdeathsigger is always forced off, or only set a default so explicit caller intent is preserved.
# Always disable pdeathsigger wrapper when running as target
# This function is called inside target namespaces where pdeathsigger may not exist
kwargs["pdeathsigger"] = False
gprofiler/metadata/versions.py:23
- get_exe_version() now unconditionally calls process.uids()/gids() even though privilege dropping only applies when running as root. On non-root runs this can raise psutil.AccessDenied for processes you can otherwise inspect/execute, regressing version detection. Consider resolving target_uid/target_gid only when is_root() is true; otherwise use dummy values (run_process_as_target will not drop privileges anyway).
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
…e 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 <noreply@anthropic.com>
mlim19
force-pushed
the
fix_privilege_escalation_binary_execution
branch
from
July 23, 2026 00:44
2e9d025 to
d33ffa4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/pdeathsigger.c:56
- If the caller provides only one of -u or -g, the program will continue and run the target command without dropping privileges (potentially still as root). Since the usage/docs require both, it’s safer to fail fast unless both are provided together.
if (arg_offset >= argc) {
fprintf(stderr, "Usage: %s [-u <uid> -g <gid>] /path/to/binary [args...]\n", argv[0]);
return 1;
}
scripts/pdeathsigger.c:45
- UID/GID parsing doesn’t validate that the numeric value fits in uid_t/gid_t before casting. On platforms where gid_t/uid_t are narrower than long, a large value could silently truncate and run under an unintended identity.
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;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_process_as_target()helper that executes commands with the target process's UID/GIDChanges
run_process_as_target()function ingprofiler/utils/__init__.pyget_exe_version()inmetadata/versions.py_get_sys_maxunicode()inprofilers/python.pyget_java_version()inprofilers/java.pyTest plan
🤖 Generated with Claude Code