What happened:
networkSetup() in pkg/network/network.go:225 creates a TAP device first, then performs a sequence of steps that can each fail independently (bringing the TAP/redirect link up, adding ingress qdiscs, adding TC redirect filters, assigning an IP). Every error branch after the TAP is created returns immediately with return nil, fmt.Errorf(...) and never calls netlink.LinkDel on the TAP it just created. The same problem exists one level up in DynamicNetwork.NetworkSetup() (pkg/network/network_dynamic.go:53-63): if networkSetup() succeeds but the subsequent getInterfaceInfo() call fails, the TAP created by networkSetup() is likewise never torn down.
Root cause:
pkg/network/network.go:225-283
func networkSetup(tapName string, ipAddress string, redirectLink netlink.Link, addTCRules bool, uid uint32, gid uint32) (netlink.Link, error) {
newTapDevice, err := createTapDevice(tapName, redirectLink.Attrs().MTU, uid, gid)
if err != nil {
return nil, fmt.Errorf("createTapDevice(%s) failed: %w", tapName, err)
}
if err = netlink.LinkSetUp(newTapDevice); err != nil {
return nil, fmt.Errorf("LinkSetUp(%s) failed: %w", newTapDevice.Attrs().Name, err) // tap leaked here, no LinkDel
}
...
if addTCRules {
if err = addIngressQdisc(newTapDevice); err != nil {
return nil, fmt.Errorf("addIngressQdisc(tap=%s) failed: %w", ...) // tap leaked
}
...
if err = addRedirectFilter(redirectLink, newTapDevice); err != nil {
return nil, fmt.Errorf(...) // tap leaked
}
}
if ipAddress != "" {
...
if err = netlink.AddrReplace(newTapDevice, ipn); err != nil {
return nil, fmt.Errorf("AddrReplace(%s, %s) failed: %w", ...) // tap leaked
}
}
return newTapDevice, nil
}
pkg/network/network_dynamic.go:53-63
newTapDevice, err := networkSetup(newTapName, "", redirectLink, true, uid, gid)
if err != nil {
return nil, fmt.Errorf("networkSetup(%s) failed: %w", newTapName, err)
}
...
ifInfo, err := getInterfaceInfo(redirectLink.Attrs().Name)
if err != nil {
return nil, fmt.Errorf("getInterfaceInfo(%s) failed: %w", redirectLink.Attrs().Name, err) // tap already created above, leaked
}
The only cleanup path in the codebase, network.CleanupAllUruncTaps() (pkg/network/network.go:287), is called exactly once, from Unikontainer.Kill() (pkg/unikontainers/unikontainers.go:833), which itself only runs on urunc delete --force (cmd/urunc/delete.go:76-80). A normal urunc delete (the path taken for an already-exited container) never invokes it.
What you expected to happen:
If any step in networkSetup() fails after the TAP device has already been created, the TAP (and any TC qdiscs/filters already attached to it) should be deleted before returning the error, so the network namespace is left clean for the next attempt.
How to reproduce:
- Get a container's netns into a state where
networkSetup() succeeds at createTapDevice/LinkSetUp but fails later, for example: the redirect link has no IPv4 address so getInterfaceInfo() in DynamicNetwork.NetworkSetup() fails, or a TC redirect filter fails because sch_ingress/act_mirred isn't available (hardened kernel/restricted node).
urunc create/start fails; a stray tapN_urunc device with TC rules attached is left in the pod's sandbox netns.
- Since a k8s pod sandbox netns persists across container restarts, retry the container start in the same pod:
getTapIndex() (pkg/network/network_dynamic.go:36) now finds the leftover tap and DynamicNetwork.NetworkSetup() unconditionally returns "unsupported operation: can't spawn multiple unikernels in the same network namespace" for every subsequent attempt, with no automatic recovery short of destroying the pod's sandbox.
Impact:
Medium-High. A single transient/environmental failure during network setup (missing IPv4 on the interface, TC module unavailable, transient netlink error) permanently and silently disables networking for that pod's netns, because the leaked TAP makes getTapIndex() > 0 on every future attempt. Recovery requires manually finding and deleting the leaked TAP, or destroying/recreating the pod, since urunc delete --force is not part of the normal restart cycle. This also means that the fix proposed in the currently open PR #865 (which turns SetupNet's previously-swallowed errors into hard failures per issue #417) will make this worse in practice: today the error is silently logged and the pod limps along without networking; once #865 merges, the same leaked TAP will make urunc create hard-fail on every subsequent attempt in that netns.
Suggested fix:
In networkSetup(), delete the just-created TAP device (netlink.LinkDel) on every error path taken after createTapDevice succeeds, most simply via a defer guarded by a named/success flag, or explicit cleanup before each early return. Apply the same treatment in DynamicNetwork.NetworkSetup() so a failure in getInterfaceInfo() after a successful networkSetup() call also tears the TAP back down. This closes the leak at its source instead of relying on the best-effort CleanupAllUruncTaps() scan that only runs on delete --force.
Environment:
- urunc version: main branch
- Hypervisor backend: any using dynamic networking (QEMU, Firecracker, etc. via
DynamicNetwork)
- OS: Linux
What happened:
networkSetup()inpkg/network/network.go:225creates a TAP device first, then performs a sequence of steps that can each fail independently (bringing the TAP/redirect link up, adding ingress qdiscs, adding TC redirect filters, assigning an IP). Every error branch after the TAP is created returns immediately withreturn nil, fmt.Errorf(...)and never callsnetlink.LinkDelon the TAP it just created. The same problem exists one level up inDynamicNetwork.NetworkSetup()(pkg/network/network_dynamic.go:53-63): ifnetworkSetup()succeeds but the subsequentgetInterfaceInfo()call fails, the TAP created bynetworkSetup()is likewise never torn down.Root cause:
pkg/network/network.go:225-283pkg/network/network_dynamic.go:53-63The only cleanup path in the codebase,
network.CleanupAllUruncTaps()(pkg/network/network.go:287), is called exactly once, fromUnikontainer.Kill()(pkg/unikontainers/unikontainers.go:833), which itself only runs onurunc delete --force(cmd/urunc/delete.go:76-80). A normalurunc delete(the path taken for an already-exited container) never invokes it.What you expected to happen:
If any step in
networkSetup()fails after the TAP device has already been created, the TAP (and any TC qdiscs/filters already attached to it) should be deleted before returning the error, so the network namespace is left clean for the next attempt.How to reproduce:
networkSetup()succeeds atcreateTapDevice/LinkSetUpbut fails later, for example: the redirect link has no IPv4 address sogetInterfaceInfo()inDynamicNetwork.NetworkSetup()fails, or a TC redirect filter fails becausesch_ingress/act_mirredisn't available (hardened kernel/restricted node).urunc create/startfails; a straytapN_uruncdevice with TC rules attached is left in the pod's sandbox netns.getTapIndex()(pkg/network/network_dynamic.go:36) now finds the leftover tap andDynamicNetwork.NetworkSetup()unconditionally returns"unsupported operation: can't spawn multiple unikernels in the same network namespace"for every subsequent attempt, with no automatic recovery short of destroying the pod's sandbox.Impact:
Medium-High. A single transient/environmental failure during network setup (missing IPv4 on the interface, TC module unavailable, transient netlink error) permanently and silently disables networking for that pod's netns, because the leaked TAP makes
getTapIndex() > 0on every future attempt. Recovery requires manually finding and deleting the leaked TAP, or destroying/recreating the pod, sinceurunc delete --forceis not part of the normal restart cycle. This also means that the fix proposed in the currently open PR #865 (which turnsSetupNet's previously-swallowed errors into hard failures per issue #417) will make this worse in practice: today the error is silently logged and the pod limps along without networking; once #865 merges, the same leaked TAP will makeurunc createhard-fail on every subsequent attempt in that netns.Suggested fix:
In
networkSetup(), delete the just-created TAP device (netlink.LinkDel) on every error path taken aftercreateTapDevicesucceeds, most simply via adeferguarded by a named/success flag, or explicit cleanup before each early return. Apply the same treatment inDynamicNetwork.NetworkSetup()so a failure ingetInterfaceInfo()after a successfulnetworkSetup()call also tears the TAP back down. This closes the leak at its source instead of relying on the best-effortCleanupAllUruncTaps()scan that only runs ondelete --force.Environment:
DynamicNetwork)