Skip to content

fix(unikontainers): reject non-positive pid in kill/signal paths#751

Open
WHOIM1205 wants to merge 1 commit into
urunc-dev:mainfrom
WHOIM1205:fix-invalid-pid-kill
Open

fix(unikontainers): reject non-positive pid in kill/signal paths#751
WHOIM1205 wants to merge 1 commit into
urunc-dev:mainfrom
WHOIM1205:fix-invalid-pid-kill

Conversation

@WHOIM1205
Copy link
Copy Markdown

Description

A partially-created unikontainer can persist a sentinel PID value (-1) in its state before the monitor process is successfully created. Subsequent lifecycle operations trusted this stored PID and passed it directly to process-control paths.

On Unix systems, kill(-1, sig) does not target a single process. Instead, it broadcasts the signal to every process the caller is permitted to signal. Since urunc typically runs with elevated privileges, a failed container creation followed by a signal or force-delete operation could result in host-wide signal delivery.

This change adds validation for non-positive PIDs at all process-control boundaries:

  • Reject signaling containers with invalid PIDs in Unikontainer.Signal()
  • Reject killing invalid PIDs in hypervisors.killProcess()
  • Treat non-positive PIDs as not running in Unikontainer.isRunning()

These checks prevent unsafe signal propagation and allow failed containers with invalid PIDs to be cleaned up correctly.

No changes to normal container lifecycle behavior. Containers with valid monitor PIDs continue to operate as before.

Related issues

  • Fixes node-wide signal broadcast caused by sentinel PID values reaching process-control paths

How was this tested?

Verified by code inspection and local validation.

  • Confirmed containers may persist Pid: -1 during the creating state
  • Verified signal and force-delete paths could previously reach unix.Kill() with non-positive PIDs
  • Confirmed invalid PIDs are now rejected before any signal is sent
  • Confirmed isRunning() returns false for invalid PIDs, allowing cleanup of failed containers
  • Ran build and vet checks successfully

LLM usage

N/A

Checklist

  • I have read the contribution guide
  • The project builds successfully after the change
  • The fix is limited to PID validation and cleanup behavior
  • No functional changes for healthy containers with valid PIDs

Signed-off-by: WHOIM1205 <rathourprateek8@gmail.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 5, 2026

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 7f6e6aa
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a231a17fcb63e00085dedf7

@WHOIM1205
Copy link
Copy Markdown
Author

Hey @cmainas this addresses the invalid PID handling issue by rejecting non-positive PIDs before any signal operation and allowing cleanup of failed containers. Please let me know if you'd like any additional tests or changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant