DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/memif: fix use after free on listener socket close
@ 2026-11-04 16:41 David Marchand
  2026-11-05  8:32 ` Stephen Hemminger
  2026-11-06 14:57 ` [PATCH v2] " David Marchand
  0 siblings, 2 replies; 6+ messages in thread
From: David Marchand @ 2026-11-04 16:41 UTC (permalink / raw)
  To: dev; +Cc: sriram.yagnaraman, jgrajcia, thomas, stephen

memif_socket_remove_device() calls rte_intr_callback_unregister() on
the listener socket interrupt handle. If memif_listener_handler() is
active at that point, the unregister returns -EAGAIN and the code
proceeds to close and free the handle while the handler is still
running.

Use rte_intr_callback_unregister_sync() which retries until the
handler completes.

Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/memif/memif_socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 971964133c..d672ad9f64 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -1198,8 +1198,8 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 					socket->filename);
 		}
 		if (pmd->role != MEMIF_ROLE_CLIENT) {
-			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
-				socket);
+			rte_intr_callback_unregister_sync(socket->intr_handle,
+				memif_listener_handler, socket);
 			rte_intr_fd_close(socket->intr_handle);
 			rte_intr_instance_free(socket->intr_handle);
 		}
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net/memif: fix use after free on listener socket close
  2026-11-04 16:41 [PATCH] net/memif: fix use after free on listener socket close David Marchand
@ 2026-11-05  8:32 ` Stephen Hemminger
  2026-11-06 14:57 ` [PATCH v2] " David Marchand
  1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2026-11-05  8:32 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, sriram.yagnaraman, jgrajcia, thomas

On Wed,  4 Nov 2026 17:41:53 +0100
David Marchand <david.marchand@redhat.com> wrote:

> memif_socket_remove_device() calls rte_intr_callback_unregister() on
> the listener socket interrupt handle. If memif_listener_handler() is
> active at that point, the unregister returns -EAGAIN and the code
> proceeds to close and free the handle while the handler is still
> running.
> 
> Use rte_intr_callback_unregister_sync() which retries until the
> handler completes.
> 
> Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Manually run AI review likes the patch but sees related pre-existing problem

Subject: Re: [PATCH] net/memif: fix use after free on listener socket close

On Wed,  4 Nov 2026 17:41:53 +0100
David Marchand <david.marchand@redhat.com> wrote:

> memif_socket_remove_device() calls rte_intr_callback_unregister() on
> the listener socket interrupt handle. If memif_listener_handler() is
> active at that point, the unregister returns -EAGAIN and the code
> proceeds to close and free the handle while the handler is still
> running.

The fix is correct. memif_socket_remove_device() is only reached from
memif_dev_close() in the primary, never from an interrupt callback,
so the _sync variant cannot spin on its own callback.

Info:

The same pattern is left in memif_socket_create_or_reuse(), on the
rte_hash_add_key_data() failure path. The listener callback was just
registered, so a client connecting in that window hits the same race.
Worth converting in the same patch:

	if (socket->listener) {
-		rte_intr_callback_unregister(socket->intr_handle,
-			memif_listener_handler, socket);
+		rte_intr_callback_unregister_sync(socket->intr_handle,
+			memif_listener_handler, socket);
		rte_intr_fd_close(socket->intr_handle);

Separate, pre-existing issue (not for this patch): control channels
accepted by memif_listener_handler() hold cc->socket but are not
tracked by the socket until memif_msg_receive_init() binds them to a
device. If the last device is closed while a client is between accept
and INIT, the socket is freed and the later INIT walks
cc->socket->dev_queue on freed memory. memif_socket_remove_device()
needs to disconnect pending control channels (or the socket needs a
list of them) before rte_free(socket).

Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] net/memif: fix use after free on listener socket close
  2026-11-04 16:41 [PATCH] net/memif: fix use after free on listener socket close David Marchand
  2026-11-05  8:32 ` Stephen Hemminger
@ 2026-11-06 14:57 ` David Marchand
  2026-11-09 22:13   ` Stephen Hemminger
  1 sibling, 1 reply; 6+ messages in thread
From: David Marchand @ 2026-11-06 14:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Sriram Yagnaraman, Jakub Grajciar

memif_socket_remove_device() calls rte_intr_callback_unregister() on
the listener socket interrupt handle. If memif_listener_handler() is
active at that point, the unregister returns -EAGAIN and the code
proceeds to close and free the handle while the handler is still
running.

Use rte_intr_callback_unregister_sync() which retries until the
handler completes.

Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
Changes since v1:
- added missing _sync in other call,

---
 drivers/net/memif/memif_socket.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c
index 971964133c..b3cd84bf21 100644
--- a/drivers/net/memif/memif_socket.c
+++ b/drivers/net/memif/memif_socket.c
@@ -1126,7 +1126,7 @@ memif_socket_init(struct rte_eth_dev *dev, const char *socket_filename)
 		if (ret < 0) {
 			MIF_LOG(ERR, "Failed to add socket to socket hash.");
 			if (socket->listener) {
-				rte_intr_callback_unregister(socket->intr_handle,
+				rte_intr_callback_unregister_sync(socket->intr_handle,
 					memif_listener_handler, socket);
 				rte_intr_fd_close(socket->intr_handle);
 				rte_intr_instance_free(socket->intr_handle);
@@ -1198,8 +1198,8 @@ memif_socket_remove_device(struct rte_eth_dev *dev)
 					socket->filename);
 		}
 		if (pmd->role != MEMIF_ROLE_CLIENT) {
-			rte_intr_callback_unregister(socket->intr_handle, memif_listener_handler,
-				socket);
+			rte_intr_callback_unregister_sync(socket->intr_handle,
+				memif_listener_handler, socket);
 			rte_intr_fd_close(socket->intr_handle);
 			rte_intr_instance_free(socket->intr_handle);
 		}
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] net/memif: fix use after free on listener socket close
  2026-11-06 14:57 ` [PATCH v2] " David Marchand
@ 2026-11-09 22:13   ` Stephen Hemminger
  2026-11-10  7:43     ` David Marchand
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2026-11-09 22:13 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Sriram Yagnaraman, Jakub Grajciar

On Fri,  6 Nov 2026 15:57:29 +0100
David Marchand <david.marchand@redhat.com> wrote:

> memif_socket_remove_device() calls rte_intr_callback_unregister() on
> the listener socket interrupt handle. If memif_listener_handler() is
> active at that point, the unregister returns -EAGAIN and the code
> proceeds to close and free the handle while the handler is still
> running.
> 
> Use rte_intr_callback_unregister_sync() which retries until the
> handler completes.
> 
> Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Changes since v1:
> - added missing _sync in other call,


Applied to next-net but dropped the Fixes line since that commit
is after the last release. Fixes is not necessary during the merge
cycle.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] net/memif: fix use after free on listener socket close
  2026-11-09 22:13   ` Stephen Hemminger
@ 2026-11-10  7:43     ` David Marchand
  2026-11-10 14:36       ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: David Marchand @ 2026-11-10  7:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Sriram Yagnaraman, Jakub Grajciar, Thomas Monjalon

On Mon, 9 Nov 2026 at 23:14, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri,  6 Nov 2026 15:57:29 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > memif_socket_remove_device() calls rte_intr_callback_unregister() on
> > the listener socket interrupt handle. If memif_listener_handler() is
> > active at that point, the unregister returns -EAGAIN and the code
> > proceeds to close and free the handle while the handler is still
> > running.
> >
> > Use rte_intr_callback_unregister_sync() which retries until the
> > handler completes.
> >
> > Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > Changes since v1:
> > - added missing _sync in other call,
>
>
> Applied to next-net but dropped the Fixes line since that commit
> is after the last release. Fixes is not necessary during the merge
> cycle.

I disagree.

Fixes: tag are not *only* for backport purpose.
Those tags also give useful information when looking back in git
history, to understand why change X is done.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] net/memif: fix use after free on listener socket close
  2026-11-10  7:43     ` David Marchand
@ 2026-11-10 14:36       ` Stephen Hemminger
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2026-11-10 14:36 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Sriram Yagnaraman, Jakub Grajciar, Thomas Monjalon

On Tue, 10 Nov 2026 08:43:28 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Mon, 9 Nov 2026 at 23:14, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Fri,  6 Nov 2026 15:57:29 +0100
> > David Marchand <david.marchand@redhat.com> wrote:
> >  
> > > memif_socket_remove_device() calls rte_intr_callback_unregister() on
> > > the listener socket interrupt handle. If memif_listener_handler() is
> > > active at that point, the unregister returns -EAGAIN and the code
> > > proceeds to close and free the handle while the handler is still
> > > running.
> > >
> > > Use rte_intr_callback_unregister_sync() which retries until the
> > > handler completes.
> > >
> > > Fixes: 7281511c2cfa ("net/memif: fix some interrupt FD leaks")
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > > Changes since v1:
> > > - added missing _sync in other call,  
> >
> >
> > Applied to next-net but dropped the Fixes line since that commit
> > is after the last release. Fixes is not necessary during the merge
> > cycle.  
> 
> I disagree.
> 
> Fixes: tag are not *only* for backport purpose.
> Those tags also give useful information when looking back in git
> history, to understand why change X is done.

Makes sense, just don't cc stable

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-10 14:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 16:41 [PATCH] net/memif: fix use after free on listener socket close David Marchand
2026-11-05  8:32 ` Stephen Hemminger
2026-11-06 14:57 ` [PATCH v2] " David Marchand
2026-11-09 22:13   ` Stephen Hemminger
2026-11-10  7:43     ` David Marchand
2026-11-10 14:36       ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox