[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

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790751008.git.shijianlin11@foxmail.com?part=1

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

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790751008.git.shijianlin11@foxmail.com?part=2

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