[PATCH 1/2] net/tap: fix FD leak on netns change

[David Marchand]

On Wed, 4 Nov 2026 at 13:21, Stephen Hemminger
<stephen at networkplumber.org> wrote:
>
> On Tue,  3 Nov 2026 18:17:29 +0100
> David Marchand <david.marchand at redhat.com> wrote:
>
> > When a tap interface moves to another namespace, tap_netns_change()
> > calls tap_lsc_intr_handle_set(dev, 1) to recreate the LSC interrupt
> > without first tearing down the old one. The previous LSC netlink
> > socket fd is never closed.
> >
> > Defer the full LSC teardown and re-registration to an alarm callback
> > which runs in the interrupt thread but outside any active source
> > context. The alarm switches to the device's network namespace, tears
> > down the old LSC handle, sets up a new one (opening a fresh netlink
> > socket in the correct namespace), restores the original namespace,
> > and forces carrier state back up.
> >
> > Extract tap_dev_ns_switch/tap_dev_ns_restore helpers to share the
> > namespace switch logic between tap_netns_change and the alarm.
> >
> > Fixes: aae3a38e9e03 ("net/tap: detect namespace change")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand at redhat.com>
>
>
> Saw some AI review issues:
>
> Review: net/tap: fix FD leak on netns change (2 patches)
>
> Series applies to main (26.11-rc2) and builds with -Dwerror=true at
> each commit. Fixes: tags are correct: aae3a38e9e03 is in v25.11, so
> Cc: stable on 1/2 is right; c663018eb7f3 is 26.11 only, so 2/2 needs
> none.
>
> Deferring the LSC re-registration to an alarm is the right approach.
> The alarm runs in the interrupt thread with no source active, so
> rte_intr_callback_unregister() completes, and the self-cancel in
> clean: returns EINPROGRESS instead of spinning.
>
>
> [PATCH 1/2] net/tap: fix FD leak on netns change
>
> Warning: setns()/open() failure now reported as "interface deleted"
>
> tap_dev_ns_switch() returns -1 for three different cases: no netns
> (interface gone), open("/proc/self/ns/net") failure, and setns()
> failure (e.g. EPERM without CAP_SYS_ADMIN). tap_netns_change() treats
> all of them as deletion, logs at INFO and returns 0. Before the patch
> the last two logged ERR with strerror(errno) and returned -1. Keep the
> logging in the helper:
>
>         *orig_netns_fd = open("/proc/self/ns/net", O_RDONLY);
>         if (*orig_netns_fd < 0) {
>                 TAP_LOG(ERR, "%s: failed to open original netns: %s",
>                         pmd->name, strerror(errno));
>                 close(netns_fd);
>                 return -1;
>         }
>
>         if (setns(netns_fd, CLONE_NEWNET) < 0) {
>                 TAP_LOG(ERR, "%s: failed to enter new netns: %s",
>                         pmd->name, strerror(errno));
>                 ...
>
> and return -ENOENT (no log) for the TUNGETDEVNETNS failure so the
> caller can tell deletion from error.

Regardless of the error, we are in an inconsistent state and the
caller can not recover.
I will add back some logs in the helper, but returning a different
error code is pointless.


> Warning: tap_dev_ns_restore() ignores setns() failure
>
> The old code logged ERR when switching back failed. Now it is silent,
> and if it fails the interrupt thread stays in the device's netns. Every
> later netlink socket opened from that thread (next LSC re-register,
> alarms) lands in the wrong namespace. Keep the error:
>
>         static void
>         tap_dev_ns_restore(struct pmd_internals *pmd, int orig_netns_fd)
>         {
>                 if (setns(orig_netns_fd, CLONE_NEWNET) < 0)
>                         TAP_LOG(ERR, "%s: failed to return to original netns: %s",
>                                 pmd->name, strerror(errno));
>                 close(orig_netns_fd);
>         }
> Info: rte_eal_alarm_set() return value unchecked
>
> If it fails the LSC socket stays in the old namespace and no further
> link events arrive, with nothing in the log.
>
>         if (rte_eal_alarm_set(1, tap_lsc_register_alarm, dev) < 0)
>                 TAP_LOG(ERR, "%s: failed to schedule LSC re-registration",
>                         pmd->name);

Original code was light on the logging side.
I will add some error logs.


> Info: concurrent dev_stop window
> clean: cancels the alarm before the -EAGAIN retry loop. If dev_stop
> runs while tap_dev_intr_handler is active, the handler can arm the
> alarm after the cancel. In practice the alarm fires during the 100 ms
> rte_delay_ms() and the retry unregisters what it installed, but if the
> alarm runs as the stop thread wakes, both threads operate on
> pmd->intr_handle fd at once. Worth a comment, or a second
> rte_eal_alarm_cancel() before the do/while retry.

Well, that looks like a real issue.
Fixed.


> [PATCH 2/2] net/tap: fix use after free on interrupt handle
>
> Info: commit message does not mention the fd < 0 early return
>
> The new check in clean: is needed because close after stop now reaches
> the unregister loop with fd == -1, where rte_intr_callback_unregister()
> returns -EINVAL and logs "Unregistering with invalid input parameter"
> plus the driver's "intr callback unregister failed". Add a sentence
> saying so.

This added check is actually misplaced and belongs to first patch.


-- 
David Marchand