Re: [PATCH net] net: ntb_netdev: Fix statistics races

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pending RX and TX entries are leaked because their associated SKB buffers are not freed when the transport queue is destroyed.
--

--- Patch [1]: [PATCH net] net: ntb_netdev: Fix statistics races ---
commit fc558d0a991ec95505df001446270417159e2eaf
Author: Koichiro Den <den@valinux.co.jp>

net: ntb_netdev: Fix statistics races

ntb_netdev updates shared net_device stats from per-QP RX and TX callbacks.
This commit shifts the driver to use core-managed per-CPU dstats. It also
tears down transport queues from ndo_uninit before the core frees dstats.

> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 9c171697e7620..6a1e58d5d7d67 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c
[ ... ]
> @@ -427,7 +428,19 @@ static int ntb_netdev_change_mtu(struct net_device *ndev, int new_mtu)
>  	return rc;
>  }
>  
> +static void ntb_netdev_uninit(struct net_device *ndev)
> +{
> +	struct ntb_netdev *dev = netdev_priv(ndev);
> +	unsigned int q;
> +
> +	for (q = 0; q < dev->num_queues; q++) {
> +		ntb_transport_free_queue(dev->queues[q].qp);

[Severity: High]
This is a pre-existing issue, but does calling ntb_transport_free_queue()
leak the SKBs attached to pending packets?

Looking at ntb_transport_free_queue() in drivers/ntb/ntb_transport.c,
it cleans up pending items from the queues like this:

drivers/ntb/ntb_transport.c:ntb_transport_free_queue() {
    ...
    while ((entry = ntb_list_rm(&qp->ntb_rx_q_lock, &qp->rx_post_q))) {
        dev_warn(&pdev->dev, "Freeing item from non-empty rx_post_q\n");
        kfree(entry);
    }
    ...
}

It frees the entry, but it doesn't seem to free or return the attached
client buffer (entry->cb_data). Since ntb_netdev uses cb_data to store
allocated SKBs, does this unconditionally leak any in-flight packets
when the queue is destroyed?

> +		dev->queues[q].qp = NULL;
> +	}
> +}
> +
>  static const struct net_device_ops ntb_netdev_ops = {
> +	.ndo_uninit = ntb_netdev_uninit,
>  	.ndo_open = ntb_netdev_open,
>  	.ndo_stop = ntb_netdev_close,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824025720.3520288-1-den@valinux.co.jp?part=1