From 80f887e452512ffd964e6d72e744e9c349f9c34f Mon Sep 17 00:00:00 2001
From: mn-ram <235066282+mn-ram@users.noreply.github.com>
Date: Wed, 29 Apr 2026 18:59:24 +0530
Subject: [PATCH] fix(kill): lock OS thread around sandbox netns join to
 prevent TAP leak

Unikontainer.Kill() called joinSandboxNetNs() (which performs
unix.Setns(CLONE_NEWNET) on the calling OS thread only) without first
calling runtime.LockOSThread(). Between the setns and the subsequent
netlink work in network.CleanupAllUruncTaps(), the Go scheduler is
free to migrate the goroutine to another M still bound to the host
network namespace. When that happens, netlink.NewHandle() inside
CleanupAllUruncTaps creates its socket in the wrong namespace, the
^tap\d+_urunc$ scan finds zero matches, and Kill() returns success
while leaving the sandbox's TAP device, ingress qdisc, and tc
redirect filters in place.

The contract was already documented on joinSandboxNetNs ("This
function should be called only from a locked thread (i.e.
runtime.LockOSThread())") and respected in Exec(); Kill() simply
missed it.

Lock the OS thread for the duration of Kill(), and snapshot/restore
the original netns so the locked thread is not returned to the
runtime pool while still pointing at the sandbox netns.

Reproducer:
  for i in $(seq 50); do
    nerdctl run -d --name uk$i --runtime io.containerd.urunc.v2 <img>
    nerdctl kill uk$i
  done
  nsenter -t <sandbox-pid> -n ip link | grep urunc | wc -l

Before: 50.  After: 0.
Signed-off-by: mn-ram <235066282+mn-ram@users.noreply.github.com>
---
 pkg/unikontainers/unikontainers.go | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go
index d63809bd..3e4f560f 100644
--- a/pkg/unikontainers/unikontainers.go
+++ b/pkg/unikontainers/unikontainers.go
@@ -560,10 +560,35 @@ func setupUser(user specs.User) error {
 // Kill stops the VMM process, first by asking the VMM struct to stop
 // and consequently by killing the process described in u.State.Pid
 func (u *Unikontainer) Kill() error {
+	// joinSandboxNetNs uses setns(CLONE_NEWNET), which mutates the netns of
+	// the calling OS thread only. Without locking the goroutine to its OS
+	// thread, the Go scheduler can migrate us to another M between the join
+	// and the netlink-backed CleanupAllUruncTaps call, causing the cleanup
+	// to run in the wrong network namespace and silently leak the sandbox's
+	// tapN_urunc devices, ingress qdiscs, and tc redirect filters.
+	// We also snapshot the original netns and restore it before unlocking,
+	// so the locked thread is not handed back to the runtime pool while
+	// still pointing at the sandbox netns.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	hostNsFd, err := unix.Open("/proc/self/ns/net", unix.O_RDONLY|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return fmt.Errorf("failed to snapshot current netns: %w", err)
+	}
+	defer func() {
+		if setnsErr := unix.Setns(hostNsFd, unix.CLONE_NEWNET); setnsErr != nil {
+			uniklog.Errorf("failed to restore original netns: %v", setnsErr)
+		}
+		if closeErr := unix.Close(hostNsFd); closeErr != nil {
+			uniklog.Errorf("failed to close netns fd: %v", closeErr)
+		}
+	}()
+
 	// Try to join the Network namespace of the monitor before killing it.
 	// If we kill it there might be no process inside the namespace and hence
 	// the namespace gets destroyed.
-	err := u.joinSandboxNetNs()
+	err = u.joinSandboxNetNs()
 	if err != nil {
 		if errors.Is(err, ErrNotExistingNS) {
 			// There is no network namespace to join.