Re: [net-next,v16,05/12] virtio_net: Query and set flow filter caps

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
virtio_net: Query and set flow filter caps

This patch adds flow filter capability support to virtio_net. It queries
and sets flow filter capabilities during probe and restore, with cleanup
during freeze and remove.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index db88dcaefb20..03bd3d2c2b0e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c

[ ... ]

> @@ -5727,10 +5934,31 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>  			return err;
>  	}
>
> +	/* Initialize flow filters. Not supported is an acceptable and common
> +	 * return code
> +	 */
> +	rtnl_lock();
> +	err = virtnet_ff_init(&vi->ff, vi->vdev);
> +	if (err && err != -EOPNOTSUPP) {
> +		if (netif_running(vi->dev))
> +			virtnet_close(vi->dev);
> +
> +		/* disable_rx_mmode_work takes the rtnl_lock, so just set the
> +		 * flag here while holding the lock.
> +		 *
> +		 * remove_vq_common resets the device and frees the vqs.
> +		 */
> +		vi->rx_mode_work_enabled = false;
> +		rtnl_unlock();
> +		remove_vq_common(vi);
> +		return err;

If virtnet_ff_init() fails here, remove_vq_common() frees vi->rq, vi->sq,
and vi->ctrl via virtnet_free_queues(), but the netdevice remains
registered. Could this leave the device in an inconsistent state where
subsequent operations (like virtnet_open() triggered by bringing the
interface up) would access freed memory through vi->rq[i]?

The error return propagates up to virtnet_restore() which just returns
the error without further cleanup. If userspace then tries to use the
still-registered netdevice, virtnet_open() would call try_fill_recv()
which dereferences vi->rq.

> +	}
> +	rtnl_unlock();
> +
>  	netif_tx_lock_bh(vi->dev);
>  	netif_device_attach(vi->dev);
>  	netif_tx_unlock_bh(vi->dev);
> -	return err;
> +	return 0;
>  }
-- 
pw-bot: cr