[PATCH 0/2] virtio-net/dim: fix irq moderation init failure unwind

[PATCH 0/2] virtio-net/dim: fix irq moderation init failure unwind

[Jianlin Shi]

net_dim_init_irq_moder() leaves dev->irq_moder dangling when profile
allocation fails.  virtio-net probe then skips virtqueue teardown if
that init fails after init_vqs().

Patch 1 makes the dim init failure path clear irq_moder, so
net_dim_free_irq_moder() is a no-op.

Patch 2 sends the virtio-net failure through the existing
free_irq_moder unwind (reset + del_vqs), matching the page-pool
failure path.

Patch 2 depends on patch 1.

Jianlin Shi (2):
  dim: clear irq_moder on init failure
  virtio-net: unwind VQs when irq moderation init fails

 drivers/net/virtio_net.c | 2 +-
 lib/dim/net_dim.c        | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/2] dim: clear irq_moder on init failure

[Jianlin Shi]

net_dim_init_irq_moder() publishes the allocated object in
dev->irq_moder before copying the moderation profiles.  If kmemdup()
fails, the object is freed but the pointer is left dangling.
net_dim_free_irq_moder() treats a non-NULL irq_moder as live, so a
caller unwinding the failure path would use-after-free.

NULL the pointer after freeing it.  On error the device is left as if
initialization never happened.

Fixes: f750dfe825b90 ("ethtool: provide customized dim profile management")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 lib/dim/net_dim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index d8d4f6553559..bf59deae6a29 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -141,6 +141,7 @@ int net_dim_init_irq_moder(struct net_device *dev, u8 profile_flags,
 	kfree(rxp);
 free_moder:
 	kfree(moder);
+	dev->irq_moder = NULL;
 	return -ENOMEM;
 }
 EXPORT_SYMBOL(net_dim_init_irq_moder);
-- 
2.43.0

[PATCH 2/2] virtio-net: unwind VQs when irq moderation init fails

[Jianlin Shi]

virtnet_probe() calls init_vqs() before virtnet_init_irq_moder().
The failure path used "goto free", which only calls free_netdev() and
skips virtio_reset_device() and virtnet_del_vqs().  The virtqueues and
their DMA mappings leak.

Use the existing free_irq_moder label, as virtnet_create_page_pools()
already does.  This is safe only after net_dim_init_irq_moder() clears
dev->irq_moder on failure; otherwise virtnet_free_irq_moder() would
free the dangling pointer.

Fixes: dcb67f6a9ead6 ("virtio-net: support dim profile fine-tuning")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e34c52d059d3..ef61097c4404 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -6996,7 +6996,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 		err = virtnet_init_irq_moder(vi);
 		if (err)
-			goto free;
+			goto free_irq_moder;
 	}
 
 	/* Create page pools for receive queues.
-- 
2.43.0

Re: [PATCH 1/2] dim: clear irq_moder on init failure

[Jakub Kicinski]

On Wed, 30 Sep 2026 16:19:51 +0800 Jianlin Shi wrote:
> net_dim_init_irq_moder() publishes the allocated object in
> dev->irq_moder before copying the moderation profiles.  If kmemdup()
> fails, the object is freed but the pointer is left dangling.
> net_dim_free_irq_moder() treats a non-NULL irq_moder as live, so a
> caller unwinding the failure path would use-after-free.
> 
> NULL the pointer after freeing it.  On error the device is left as if
> initialization never happened.


Please add an example sequence of calls in the current source tree
where this is a problem. Dangling pointers are not an issue unless
something tries to deref them.
-- 
pw-bot: cr

Re: [PATCH 1/2] dim: clear irq_moder on init failure

[Jianlin Shi]

You're right - there is no such sequence on current mainline.

net_dim_init_irq_moder() has a single in-tree caller (virtio-net). If it
fails, virtnet_probe() still does "goto free", which only ends in
free_netdev(). Nothing calls net_dim_free_irq_moder() on that path, so
the stale dev->irq_moder value is never dereferenced; it disappears when
the net_device is freed.

The dereference becomes reachable with the unwind change in patch 2:

  virtnet_init_irq_moder()
    -> net_dim_init_irq_moder()  /* fails after kfree(moder); irq_moder stale */
  goto free_irq_moder;            /* patch 2 */
    virtnet_free_irq_moder()
      net_dim_free_irq_moder()
        /* non-NULL irq_moder => */
        rtnl_dereference(dev->irq_moder->rx_profile)  /* UAF */
        kfree(dev->irq_moder)                         /* freed again */

So patch 1 is not fixing a reachable bug on today's tree by itself; it is
a hard prerequisite for patch 2. Without patch 1, patch 2 would trade the
virtqueue leak for UAF (and a second kfree of moder) on this error path.

I can add this call chain to the patch 1 commit message in v2 if you prefer.

Jianlin

Re: [PATCH 2/2] virtio-net: unwind VQs when irq moderation init fails

[Michael S. Tsirkin]

On Wed, Sep 30, 2026 at 04:19:52PM +0800, Jianlin Shi wrote:
> virtnet_probe() calls init_vqs() before virtnet_init_irq_moder().
> The failure path used "goto free", which only calls free_netdev() and
> skips virtio_reset_device() and virtnet_del_vqs().  The virtqueues and
> their DMA mappings leak.
> 
> Use the existing free_irq_moder label, as virtnet_create_page_pools()
> already does.  This is safe only after net_dim_init_irq_moder() clears
> dev->irq_moder on failure; otherwise virtnet_free_irq_moder() would
> free the dangling pointer.
> 
> Fixes: dcb67f6a9ead6 ("virtio-net: support dim profile fine-tuning")
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>


I expect v2 of this.


> ---
>  drivers/net/virtio_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index e34c52d059d3..ef61097c4404 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -6996,7 +6996,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>  
>  		err = virtnet_init_irq_moder(vi);
>  		if (err)
> -			goto free;
> +			goto free_irq_moder;
>  	}
>  
>  	/* Create page pools for receive queues.
> -- 
> 2.43.0

[PATCH v2 0/2] virtio-net/dim: fix irq moderation init failure unwind

[Jianlin Shi]

net_dim_init_irq_moder() leaves dev->irq_moder dangling when
profile allocation fails.  virtio-net probe then skips the
virtqueue teardown if that init fails after init_vqs(), leaking
the virtqueues and their DMA mappings.

Patch 1 makes the dim init failure path clear dev->irq_moder, so
net_dim_free_irq_moder() is a no-op afterwards.  Patch 2 sends the
virtio-net failure through the existing free_irq_moder unwind
(reset + del_vqs), matching the page-pool failure path.  Patch 2
depends on patch 1.

Changes since v1 [1]:

- Patch 1's commit message now documents the call sequence the
  maintainer asked for: on current mainline the dangling pointer
  is never dereferenced by the only in-tree caller (virtio-net
  probe does "goto free", which only reaches free_netdev()).  It
  becomes reachable once the caller unwinds through
  free_irq_moder as patch 2 does, which makes patch 1 a hard
  prerequisite rather than a standalone bug fix.

- No functional changes; both patches are identical to v1.

[1] https://lore.kernel.org/netdev/cover.1790751008.git.shijianlin11@foxmail.com/

Jianlin Shi (2):
  dim: clear irq_moder on init failure
  virtio-net: unwind VQs when irq moderation init fails

 drivers/net/virtio_net.c | 2 +-
 lib/dim/net_dim.c        | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>

--
2.43.0

[PATCH v2 1/2] dim: clear irq_moder on init failure

[Jianlin Shi]

net_dim_init_irq_moder() publishes the allocated struct via
dev->irq_moder before copying the moderation profiles.  If a
kmemdup() fails, the error path frees the object but leaves
dev->irq_moder dangling.  net_dim_free_irq_moder() treats a
non-NULL irq_moder as live: it dereferences
dev->irq_moder->rx_profile / tx_profile under RTNL and frees the
object a second time.

On current mainline the only in-tree caller of
net_dim_init_irq_moder() is virtnet_probe(), and its failure path
today does "goto free", which only reaches free_netdev(); nothing
calls net_dim_free_irq_moder() there, so the dangling pointer is
never dereferenced.  The bug becomes reachable once the caller
unwinds through the existing free_irq_moder label, as virtio-net
starts doing in the follow-up fix:

    virtnet_probe()
      virtnet_init_irq_moder()
        net_dim_init_irq_moder()    /* kmemdup fails:
                                       kfree(moder),
                                       dev->irq_moder stale */
      goto free_irq_moder
      virtnet_free_irq_moder()
        net_dim_free_irq_moder()    /* non-NULL irq_moder =>
                                       rtnl_dereference(
                                         dev->irq_moder->rx_profile)
                                       use-after-free,
                                       kfree() a second time */

This patch is the prerequisite that makes such an unwind safe;
without it the follow-up would trade the virtqueue leak for a
use-after-free and a double kfree on this error path.

NULL the pointer after freeing it, so a later
net_dim_free_irq_moder() is a no-op and the device is left as if
initialization never happened.

Fixes: f750dfe825b90 ("ethtool: provide customized dim profile management")
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 lib/dim/net_dim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index d8d4f6553559..bf59deae6a29 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -141,6 +141,7 @@ int net_dim_init_irq_moder(struct net_device *dev, u8 profile_flags,
 	kfree(rxp);
 free_moder:
 	kfree(moder);
+	dev->irq_moder = NULL;
 	return -ENOMEM;
 }
 EXPORT_SYMBOL(net_dim_init_irq_moder);
-- 
2.43.0

[PATCH v2 2/2] virtio-net: unwind VQs when irq moderation init fails

[Jianlin Shi]

virtnet_probe() calls init_vqs() before virtnet_init_irq_moder().
When the irq moderation init fails, the error path does "goto
free", which only reaches free_netdev(): virtio_reset_device() and
virtnet_del_vqs() are skipped, leaking the virtqueues and their
DMA mappings.

Use the existing free_irq_moder label instead, as
virtnet_create_page_pools() already does on failure.  This is safe
only after net_dim_init_irq_moder() clears dev->irq_moder on its
own failure (previous patch); otherwise virtnet_free_irq_moder()
would net_dim_free_irq_moder() a dangling pointer, dereferencing
freed memory and freeing it a second time.

Fixes: dcb67f6a9ead6 ("virtio-net: support dim profile fine-tuning")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dd2c41ef6fbe..d3ad1f1334bb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -7027,7 +7027,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 		err = virtnet_init_irq_moder(vi);
 		if (err)
-			goto free;
+			goto free_irq_moder;
 	}
 
 	/* Create page pools for receive queues.
-- 
2.43.0