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

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri,  6 Nov 2026 09:56:59 +0100
David Marchand <david.marchand@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@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Robin Jarry <rjarry@redhat.com>
> ---

AI review still seeing errors.

Review of [PATCH v2 0/2] net/tap: FD leak and interrupt UAF fixes

Applied to main (v26.11-rc2), both commits build with -Dwerror=true.

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

Error

The deferred alarm can run concurrently with the LSC teardown done by
dev_stop/dev_close and re-register the callback after the control
thread has removed it. The second rte_eal_alarm_cancel() added in v2
waits for a running alarm but does not undo what it did.

  ctrl thread (dev_stop)            intr thread
  ----------------------            -----------
  rte_eal_alarm_cancel() /* none */
                                    tap_dev_intr_handler()
                                      tap_netns_change()
                                        rte_eal_alarm_set(1, ...)
  rte_intr_callback_unregister()
    -> success (not active)
                                    tap_lsc_register_alarm()
                                      tap_lsc_intr_handle_set(dev, 0)
                                        unregister -> -ENOENT
                                        close old fd, fd = -1
                                      tap_lsc_intr_handle_set(dev, 1)
                                        new fd, callback registered
  rte_eal_alarm_cancel()
    /* spins until alarm done, returns 0 */
  tap_nl_final(new fd); fd = -1

The port is left stopped with tap_dev_intr_handler still registered on
pmd->intr_handle with fd -1, and the carrier forced up. A following
tap_dev_close() then does rte_intr_instance_free() with the callback
still registered, which is the use after free patch 2 fixes.

Repeat the teardown until the fd is stable once no alarm is pending or
running, and treat -ENOENT as already unregistered:

clean:
	for (;;) {
		rte_eal_alarm_cancel(tap_lsc_register_alarm, dev);
		fd = rte_intr_fd_get(pmd->intr_handle);
		if (fd < 0)
			return 0;

		do {
			ret = rte_intr_callback_unregister(pmd->intr_handle,
				tap_dev_intr_handler, dev);
			if (ret >= 0 || ret == -ENOENT)
				break;
			if (ret != -EAGAIN) {
				TAP_LOG(ERR, "intr callback unregister failed: %d",
					ret);
				break;
			}
			rte_delay_ms(100);
		} while (true);

		/* alarm may have replaced the fd while unregistering */
		rte_eal_alarm_cancel(tap_lsc_register_alarm, dev);
		if (rte_intr_fd_get(pmd->intr_handle) == fd)
			break;
	}

	tap_nl_final(fd);
	/* fd has been closed already, only clear reference here. */
	rte_intr_fd_set(pmd->intr_handle, -1);
	return 0;

When called from the alarm itself the cancel returns with EINPROGRESS,
the fd does not change, and the loop exits on the first pass.

Review-Result: ERROR