* [PATCH 2/2] net/tap: fix use after free on interrupt handle
2026-11-03 17:17 [PATCH 1/2] net/tap: fix FD leak on netns change David Marchand
@ 2026-11-03 17:17 ` David Marchand
2026-11-04 10:31 ` Robin Jarry
2026-11-04 10:31 ` [PATCH 1/2] net/tap: fix FD leak on netns change Robin Jarry
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: David Marchand @ 2026-11-03 17:17 UTC (permalink / raw)
To: dev; +Cc: rjarry, stephen
This driver was not unregistering its interrupt callbacks when closing
the port, which now means use after free after the interrupt handle
is not duplicated anymore by the interrupt thread.
Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
drivers/net/tap/rte_eth_tap.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 15320c4b7e..15dca58c8d 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1106,6 +1106,8 @@ tap_dev_close(struct rte_eth_dev *dev)
return 0;
}
+ tap_intr_handle_set(dev, 0);
+
if (!internals->persist)
tap_link_set_down(dev);
@@ -1958,6 +1960,9 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
clean:
rte_eal_alarm_cancel(tap_lsc_register_alarm, dev);
+ if (rte_intr_fd_get(pmd->intr_handle) < 0)
+ return 0;
+
do {
ret = rte_intr_callback_unregister(pmd->intr_handle,
tap_dev_intr_handler, dev);
--
2.56.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/2] net/tap: fix use after free on interrupt handle
2026-11-03 17:17 ` [PATCH 2/2] net/tap: fix use after free on interrupt handle David Marchand
@ 2026-11-04 10:31 ` Robin Jarry
0 siblings, 0 replies; 14+ messages in thread
From: Robin Jarry @ 2026-11-04 10:31 UTC (permalink / raw)
To: David Marchand, dev; +Cc: stephen
David Marchand, Nov 03, 2026 at 18:17:
> This driver was not unregistering its interrupt callbacks when closing
> the port, which now means use after free after the interrupt handle
> is not duplicated anymore by the interrupt thread.
>
> Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
Reviewed-by: Robin Jarry <rjarry@redhat.com>
Tested-by: Robin Jarry <rjarry@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] net/tap: fix FD leak on netns change
2026-11-03 17:17 [PATCH 1/2] net/tap: fix FD leak on netns change David Marchand
2026-11-03 17:17 ` [PATCH 2/2] net/tap: fix use after free on interrupt handle David Marchand
@ 2026-11-04 10:31 ` Robin Jarry
2026-11-04 12:21 ` Stephen Hemminger
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Robin Jarry @ 2026-11-04 10:31 UTC (permalink / raw)
To: David Marchand, dev; +Cc: stephen, stable
David Marchand, Nov 03, 2026 at 18:17:
> 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>
Tested-by: Robin Jarry <rjarry@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] net/tap: fix FD leak on netns change
2026-11-03 17:17 [PATCH 1/2] net/tap: fix FD leak on netns change David Marchand
2026-11-03 17:17 ` [PATCH 2/2] net/tap: fix use after free on interrupt handle David Marchand
2026-11-04 10:31 ` [PATCH 1/2] net/tap: fix FD leak on netns change Robin Jarry
@ 2026-11-04 12:21 ` Stephen Hemminger
2026-11-05 14:45 ` David Marchand
2026-11-06 8:56 ` [PATCH v2 " David Marchand
2026-11-10 10:03 ` [PATCH v3 " David Marchand
4 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2026-11-04 12:21 UTC (permalink / raw)
To: David Marchand; +Cc: dev, rjarry, stable
On Tue, 3 Nov 2026 18:17:29 +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>
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.
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);
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.
[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.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] net/tap: fix FD leak on netns change
2026-11-04 12:21 ` Stephen Hemminger
@ 2026-11-05 14:45 ` David Marchand
0 siblings, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-11-05 14:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, rjarry, stable
On Wed, 4 Nov 2026 at 13:21, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 3 Nov 2026 18:17:29 +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>
>
>
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] net/tap: fix FD leak on netns change
2026-11-03 17:17 [PATCH 1/2] net/tap: fix FD leak on netns change David Marchand
` (2 preceding siblings ...)
2026-11-04 12:21 ` Stephen Hemminger
@ 2026-11-06 8:56 ` David Marchand
2026-11-06 8:57 ` [PATCH v2 2/2] net/tap: fix use after free on interrupt handle David Marchand
` (2 more replies)
2026-11-10 10:03 ` [PATCH v3 " David Marchand
4 siblings, 3 replies; 14+ messages in thread
From: David Marchand @ 2026-11-06 8:56 UTC (permalink / raw)
To: dev; +Cc: rjarry, stable, Stephen Hemminger
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>
---
Changes since v1:
- moved pmd->if_index clearing before trying to change netns,
- moved alarm registration after netns changes succeed,
- added second alarm cancel for unregister race with netns change,
- ignored ENOENT suggestion from AI review,
- added error logs to please AI review,
---
drivers/net/tap/rte_eth_tap.c | 99 +++++++++++++++++++++++++----------
1 file changed, 72 insertions(+), 27 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 1d2b71675a..66a55301e7 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -17,6 +17,7 @@
#include <rte_ip.h>
#include <rte_ethdev.h>
#include <rte_errno.h>
+#include <rte_alarm.h>
#include <rte_cycles.h>
#include <assert.h>
@@ -1796,26 +1797,23 @@ tap_mac_addr_remove(struct rte_eth_dev *dev __rte_unused,
static void tap_dev_intr_handler(void *cb_arg);
static int tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set);
+static void tap_lsc_register_alarm(void *arg);
+#ifdef TUNGETDEVNETNS
static int
-tap_netns_change(struct rte_eth_dev *dev)
+tap_dev_ns_switch(struct pmd_internals *pmd, int *orig_netns_fd)
{
- struct pmd_internals *pmd = dev->data->dev_private;
-#ifdef TUNGETDEVNETNS
- int netns_fd, orig_netns_fd, new_nlsk_fd;
+ int netns_fd;
netns_fd = ioctl(pmd->ka_fd, TUNGETDEVNETNS);
if (netns_fd < 0) {
TAP_LOG(INFO, "%s: interface deleted", pmd->name);
- return 0;
+ return -1;
}
- /* Interface was moved to another namespace */
- pmd->if_index = 0;
-
/* Save current namespace */
- orig_netns_fd = open("/proc/self/ns/net", O_RDONLY);
- if (orig_netns_fd < 0) {
+ *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);
@@ -1827,10 +1825,37 @@ tap_netns_change(struct rte_eth_dev *dev)
TAP_LOG(ERR, "%s: failed to enter new netns: %s",
pmd->name, strerror(errno));
close(netns_fd);
- close(orig_netns_fd);
+ close(*orig_netns_fd);
return -1;
}
+ close(netns_fd);
+ return 0;
+}
+
+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 restore original netns: %s",
+ pmd->name, strerror(errno));
+ close(orig_netns_fd);
+}
+#endif
+
+static int
+tap_netns_change(struct rte_eth_dev *dev)
+{
+ struct pmd_internals *pmd = dev->data->dev_private;
+#ifdef TUNGETDEVNETNS
+ int orig_netns_fd, new_nlsk_fd;
+
+ /* Interface was moved to another namespace */
+ pmd->if_index = 0;
+
+ if (tap_dev_ns_switch(pmd, &orig_netns_fd) < 0)
+ return 0;
+
/*
* Update ifindex by querying interface name.
* The interface now has a new ifindex in the new namespace.
@@ -1840,22 +1865,7 @@ tap_netns_change(struct rte_eth_dev *dev)
/* Recreate netlink socket in new namespace */
new_nlsk_fd = tap_nl_init(0);
- /* Recreate LSC interrupt netlink socket in new namespace */
- rte_intr_callback_unregister_pending(pmd->intr_handle, tap_dev_intr_handler, dev, NULL);
- if (tap_lsc_intr_handle_set(dev, 1) < 0)
- TAP_LOG(WARNING, "%s: failed to recreate LSC interrupt socket",
- pmd->name);
-
- /* Force carrier back after switching netns */
- tap_carrier_set(pmd, 1);
-
- /* Switch back to original namespace */
- 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);
- close(netns_fd);
+ tap_dev_ns_restore(pmd, orig_netns_fd);
if (pmd->if_index == 0) {
TAP_LOG(WARNING, "%s: interface moved to another namespace, "
@@ -1877,6 +1887,11 @@ tap_netns_change(struct rte_eth_dev *dev)
tap_nl_final(pmd->nlsk_fd);
pmd->nlsk_fd = new_nlsk_fd;
+ /* Defer LSC re-registration outside callback context */
+ if (rte_eal_alarm_set(1, tap_lsc_register_alarm, dev) < 0)
+ TAP_LOG(ERR, "%s: failed to schedule LSC re-registration",
+ pmd->name);
+
TAP_LOG(INFO, "%s: interface moved to another namespace, new ifindex: %u",
pmd->name, pmd->if_index);
#else
@@ -1952,6 +1967,10 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
}
clean:
+ rte_eal_alarm_cancel(tap_lsc_register_alarm, dev);
+ if (rte_intr_fd_get(pmd->intr_handle) < 0)
+ return 0;
+
do {
ret = rte_intr_callback_unregister(pmd->intr_handle,
tap_dev_intr_handler, dev);
@@ -1966,6 +1985,8 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
}
} while (true);
+ rte_eal_alarm_cancel(tap_lsc_register_alarm, dev);
+
if (rte_intr_fd_get(pmd->intr_handle) >= 0) {
tap_nl_final(rte_intr_fd_get(pmd->intr_handle));
/* fd has been closed already, only clear reference here. */
@@ -1975,6 +1996,30 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
return 0;
}
+static void
+tap_lsc_register_alarm(void *arg)
+{
+#ifdef TUNGETDEVNETNS
+ struct rte_eth_dev *dev = arg;
+ struct pmd_internals *pmd = dev->data->dev_private;
+ int orig_netns_fd;
+
+ if (tap_dev_ns_switch(pmd, &orig_netns_fd) < 0)
+ return;
+
+ tap_lsc_intr_handle_set(dev, 0);
+ if (tap_lsc_intr_handle_set(dev, 1) < 0)
+ TAP_LOG(ERR, "%s: failed to register LSC interrupt", pmd->name);
+
+ tap_dev_ns_restore(pmd, orig_netns_fd);
+
+ tap_carrier_set(pmd, 1);
+ tap_link_update(dev, 0);
+#else
+ RTE_SET_USED(arg);
+#endif
+}
+
static int
tap_intr_handle_set(struct rte_eth_dev *dev, int set)
{
--
2.56.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 2/2] net/tap: fix use after free on interrupt handle
2026-11-06 8:56 ` [PATCH v2 " David Marchand
@ 2026-11-06 8:57 ` David Marchand
2026-11-06 11:28 ` Robin Jarry
2026-11-06 11:28 ` [PATCH v2 1/2] net/tap: fix FD leak on netns change Robin Jarry
2026-11-09 18:19 ` Stephen Hemminger
2 siblings, 1 reply; 14+ messages in thread
From: David Marchand @ 2026-11-06 8:57 UTC (permalink / raw)
To: dev; +Cc: rjarry, Stephen Hemminger, Anatoly Burakov
This driver was not unregistering its interrupt callbacks when closing
the port, which now means use after free after the interrupt handle
is not duplicated anymore by the interrupt thread.
Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Robin Jarry <rjarry@redhat.com>
---
Changes since v1:
- moved unrelated change,
---
drivers/net/tap/rte_eth_tap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 66a55301e7..f6502c5e56 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1106,6 +1106,8 @@ tap_dev_close(struct rte_eth_dev *dev)
return 0;
}
+ tap_intr_handle_set(dev, 0);
+
if (!internals->persist)
tap_link_set_down(dev);
--
2.56.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/2] net/tap: fix use after free on interrupt handle
2026-11-06 8:57 ` [PATCH v2 2/2] net/tap: fix use after free on interrupt handle David Marchand
@ 2026-11-06 11:28 ` Robin Jarry
0 siblings, 0 replies; 14+ messages in thread
From: Robin Jarry @ 2026-11-06 11:28 UTC (permalink / raw)
To: David Marchand, dev; +Cc: Stephen Hemminger, Anatoly Burakov
David Marchand, Nov 06, 2026 at 09:57:
> This driver was not unregistering its interrupt callbacks when closing
> the port, which now means use after free after the interrupt handle
> is not duplicated anymore by the interrupt thread.
>
> Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Robin Jarry <rjarry@redhat.com>
> ---
> Changes since v1:
> - moved unrelated change,
>
> ---
> drivers/net/tap/rte_eth_tap.c | 2 ++
> 1 file changed, 2 insertions(+)
Tested-by: Robin Jarry <rjarry@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] net/tap: fix FD leak on netns change
2026-11-06 8:56 ` [PATCH v2 " David Marchand
2026-11-06 8:57 ` [PATCH v2 2/2] net/tap: fix use after free on interrupt handle David Marchand
@ 2026-11-06 11:28 ` Robin Jarry
2026-11-09 18:19 ` Stephen Hemminger
2 siblings, 0 replies; 14+ messages in thread
From: Robin Jarry @ 2026-11-06 11:28 UTC (permalink / raw)
To: David Marchand, dev; +Cc: stable, Stephen Hemminger
David Marchand, Nov 06, 2026 at 09:56:
> 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>
> ---
> Changes since v1:
> - moved pmd->if_index clearing before trying to change netns,
> - moved alarm registration after netns changes succeed,
> - added second alarm cancel for unregister race with netns change,
> - ignored ENOENT suggestion from AI review,
> - added error logs to please AI review,
>
> ---
> drivers/net/tap/rte_eth_tap.c | 99 +++++++++++++++++++++++++----------
> 1 file changed, 72 insertions(+), 27 deletions(-)
Tested-by: Robin Jarry <rjarry@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/2] net/tap: fix FD leak on netns change
2026-11-06 8:56 ` [PATCH v2 " David Marchand
2026-11-06 8:57 ` [PATCH v2 2/2] net/tap: fix use after free on interrupt handle David Marchand
2026-11-06 11:28 ` [PATCH v2 1/2] net/tap: fix FD leak on netns change Robin Jarry
@ 2026-11-09 18:19 ` Stephen Hemminger
2 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2026-11-09 18:19 UTC (permalink / raw)
To: David Marchand; +Cc: dev, rjarry, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] net/tap: fix FD leak on netns change
2026-11-03 17:17 [PATCH 1/2] net/tap: fix FD leak on netns change David Marchand
` (3 preceding siblings ...)
2026-11-06 8:56 ` [PATCH v2 " David Marchand
@ 2026-11-10 10:03 ` David Marchand
2026-11-10 10:03 ` [PATCH v3 2/2] net/tap: fix use after free on interrupt handle David Marchand
2026-11-10 16:54 ` [PATCH v3 1/2] net/tap: fix FD leak on netns change Stephen Hemminger
4 siblings, 2 replies; 14+ messages in thread
From: David Marchand @ 2026-11-10 10:03 UTC (permalink / raw)
To: dev; +Cc: rjarry, stable, Stephen Hemminger
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>
---
Changes since v2:
- added retry loop for the unregister retry loop,
Changes since v1:
- moved pmd->if_index clearing before trying to change netns,
- moved alarm registration after netns changes succeed,
- added second alarm cancel for unregister race with netns change,
- ignored ENOENT suggestion from AI review,
- added error logs to please AI review,
---
drivers/net/tap/rte_eth_tap.c | 130 +++++++++++++++++++++++-----------
1 file changed, 89 insertions(+), 41 deletions(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 1d2b71675a..87779710a7 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -17,6 +17,7 @@
#include <rte_ip.h>
#include <rte_ethdev.h>
#include <rte_errno.h>
+#include <rte_alarm.h>
#include <rte_cycles.h>
#include <assert.h>
@@ -1796,26 +1797,23 @@ tap_mac_addr_remove(struct rte_eth_dev *dev __rte_unused,
static void tap_dev_intr_handler(void *cb_arg);
static int tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set);
+static void tap_lsc_register_alarm(void *arg);
+#ifdef TUNGETDEVNETNS
static int
-tap_netns_change(struct rte_eth_dev *dev)
+tap_dev_ns_switch(struct pmd_internals *pmd, int *orig_netns_fd)
{
- struct pmd_internals *pmd = dev->data->dev_private;
-#ifdef TUNGETDEVNETNS
- int netns_fd, orig_netns_fd, new_nlsk_fd;
+ int netns_fd;
netns_fd = ioctl(pmd->ka_fd, TUNGETDEVNETNS);
if (netns_fd < 0) {
TAP_LOG(INFO, "%s: interface deleted", pmd->name);
- return 0;
+ return -1;
}
- /* Interface was moved to another namespace */
- pmd->if_index = 0;
-
/* Save current namespace */
- orig_netns_fd = open("/proc/self/ns/net", O_RDONLY);
- if (orig_netns_fd < 0) {
+ *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);
@@ -1827,10 +1825,37 @@ tap_netns_change(struct rte_eth_dev *dev)
TAP_LOG(ERR, "%s: failed to enter new netns: %s",
pmd->name, strerror(errno));
close(netns_fd);
- close(orig_netns_fd);
+ close(*orig_netns_fd);
return -1;
}
+ close(netns_fd);
+ return 0;
+}
+
+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 restore original netns: %s",
+ pmd->name, strerror(errno));
+ close(orig_netns_fd);
+}
+#endif
+
+static int
+tap_netns_change(struct rte_eth_dev *dev)
+{
+ struct pmd_internals *pmd = dev->data->dev_private;
+#ifdef TUNGETDEVNETNS
+ int orig_netns_fd, new_nlsk_fd;
+
+ /* Interface was moved to another namespace */
+ pmd->if_index = 0;
+
+ if (tap_dev_ns_switch(pmd, &orig_netns_fd) < 0)
+ return 0;
+
/*
* Update ifindex by querying interface name.
* The interface now has a new ifindex in the new namespace.
@@ -1840,22 +1865,7 @@ tap_netns_change(struct rte_eth_dev *dev)
/* Recreate netlink socket in new namespace */
new_nlsk_fd = tap_nl_init(0);
- /* Recreate LSC interrupt netlink socket in new namespace */
- rte_intr_callback_unregister_pending(pmd->intr_handle, tap_dev_intr_handler, dev, NULL);
- if (tap_lsc_intr_handle_set(dev, 1) < 0)
- TAP_LOG(WARNING, "%s: failed to recreate LSC interrupt socket",
- pmd->name);
-
- /* Force carrier back after switching netns */
- tap_carrier_set(pmd, 1);
-
- /* Switch back to original namespace */
- 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);
- close(netns_fd);
+ tap_dev_ns_restore(pmd, orig_netns_fd);
if (pmd->if_index == 0) {
TAP_LOG(WARNING, "%s: interface moved to another namespace, "
@@ -1877,6 +1887,11 @@ tap_netns_change(struct rte_eth_dev *dev)
tap_nl_final(pmd->nlsk_fd);
pmd->nlsk_fd = new_nlsk_fd;
+ /* Defer LSC re-registration outside callback context */
+ if (rte_eal_alarm_set(1, tap_lsc_register_alarm, dev) < 0)
+ TAP_LOG(ERR, "%s: failed to schedule LSC re-registration",
+ pmd->name);
+
TAP_LOG(INFO, "%s: interface moved to another namespace, new ifindex: %u",
pmd->name, pmd->if_index);
#else
@@ -1929,6 +1944,7 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
{
struct pmd_internals *pmd = dev->data->dev_private;
int ret;
+ int fd;
/* In any case, disable interrupt if the conf is no longer there. */
if (!dev->data->dev_conf.intr_conf.lsc) {
@@ -1953,28 +1969,60 @@ tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set)
clean:
do {
- ret = rte_intr_callback_unregister(pmd->intr_handle,
- tap_dev_intr_handler, dev);
- if (ret >= 0) {
- break;
- } else if (ret == -EAGAIN) {
+ 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);
- } else {
- TAP_LOG(ERR, "intr callback unregister failed: %d",
- ret);
+ } 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;
- }
} while (true);
- if (rte_intr_fd_get(pmd->intr_handle) >= 0) {
- tap_nl_final(rte_intr_fd_get(pmd->intr_handle));
- /* fd has been closed already, only clear reference here. */
- rte_intr_fd_set(pmd->intr_handle, -1);
- }
+ tap_nl_final(fd);
+ /* fd has been closed already, only clear reference here. */
+ rte_intr_fd_set(pmd->intr_handle, -1);
return 0;
}
+static void
+tap_lsc_register_alarm(void *arg)
+{
+#ifdef TUNGETDEVNETNS
+ struct rte_eth_dev *dev = arg;
+ struct pmd_internals *pmd = dev->data->dev_private;
+ int orig_netns_fd;
+
+ if (tap_dev_ns_switch(pmd, &orig_netns_fd) < 0)
+ return;
+
+ tap_lsc_intr_handle_set(dev, 0);
+ if (tap_lsc_intr_handle_set(dev, 1) < 0)
+ TAP_LOG(ERR, "%s: failed to register LSC interrupt", pmd->name);
+
+ tap_dev_ns_restore(pmd, orig_netns_fd);
+
+ tap_carrier_set(pmd, 1);
+ tap_link_update(dev, 0);
+#else
+ RTE_SET_USED(arg);
+#endif
+}
+
static int
tap_intr_handle_set(struct rte_eth_dev *dev, int set)
{
--
2.56.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/2] net/tap: fix use after free on interrupt handle
2026-11-10 10:03 ` [PATCH v3 " David Marchand
@ 2026-11-10 10:03 ` David Marchand
2026-11-10 16:54 ` [PATCH v3 1/2] net/tap: fix FD leak on netns change Stephen Hemminger
1 sibling, 0 replies; 14+ messages in thread
From: David Marchand @ 2026-11-10 10:03 UTC (permalink / raw)
To: dev; +Cc: rjarry, Stephen Hemminger, Anatoly Burakov
This driver was not unregistering its interrupt callbacks when closing
the port, which now means use after free after the interrupt handle
is not duplicated anymore by the interrupt thread.
Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Robin Jarry <rjarry@redhat.com>
---
Changes since v1:
- moved unrelated change,
---
drivers/net/tap/rte_eth_tap.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 87779710a7..0476a327a1 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1106,6 +1106,8 @@ tap_dev_close(struct rte_eth_dev *dev)
return 0;
}
+ tap_intr_handle_set(dev, 0);
+
if (!internals->persist)
tap_link_set_down(dev);
--
2.56.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 1/2] net/tap: fix FD leak on netns change
2026-11-10 10:03 ` [PATCH v3 " David Marchand
2026-11-10 10:03 ` [PATCH v3 2/2] net/tap: fix use after free on interrupt handle David Marchand
@ 2026-11-10 16:54 ` Stephen Hemminger
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2026-11-10 16:54 UTC (permalink / raw)
To: David Marchand; +Cc: dev, rjarry, stable
On Tue, 10 Nov 2026 11:03:07 +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>
> ---
Looks good applied to next-net
^ permalink raw reply [flat|nested] 14+ messages in thread