netdev - Re: [net-next,03/10] idpf: move queue resources to idpf_q_vec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260118022758.1106078-1-kuba@kernel.org>
Date: Sat, 17 Jan 2026 18:27:58 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: anthony.l.nguyen@...el.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	edumazet@...gle.com,
	davem@...emloft.net,
	aleksandr.loktionov@...el.com,
	netdev@...r.kernel.org,
	Samuel.salin@...el.com,
	pabeni@...hat.com,
	madhu.chittim@...el.com,
	andrew+netdev@...n.ch,
	milena.olech@...el.com,
	joshua.a.hay@...el.com
Subject: Re: [net-next,03/10] idpf: move queue resources to idpf_q_vec_rsrc structure

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
---
idpf: move queue resources to idpf_q_vec_rsrc structure

This patch moves TX and RX queue resources from struct idpf_vport to
the newly introduced idpf_q_vec_rsrc structure as part of a refactoring
effort.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 48557c67dda8..25c15d2990d6 100644

[ ... ]

> @@ -438,9 +440,10 @@ static void idpf_rx_buf_rel_all(struct idpf_rx_queue *rxq)
>   *
>   * Free a specific rx queue resources
>   */
> -static void idpf_rx_desc_rel(struct idpf_rx_queue *rxq, struct device *dev,
> -			     u32 model)
> +static void idpf_rx_desc_rel(struct idpf_rx_queue *rxq, u32 model)
>  {
> +	struct device *dev;
> +
>  	if (!rxq)
>  		return;
>

[ ... ]

>  	if (!rxq->desc_ring)
>  		return;
>
> +	dev = &rxq->q_vector->vport->adapter->pdev->dev;
                 ^^^^^^^^^

Is this safe when q_vector has not been initialized yet? Looking at
idpf_vport_open(), the call sequence is:

  idpf_vport_queues_alloc()  -> allocates desc_ring
  idpf_vport_queue_ids_init() -> can fail, goes to queues_rel
  idpf_vport_intr_init()      -> sets q_vector via
                                 idpf_vport_intr_map_vector_to_qs()

If idpf_vport_queue_ids_init() fails, the error path jumps to queues_rel
which calls idpf_vport_queues_rel() -> idpf_rx_desc_rel_all() ->
idpf_rx_desc_rel(). At this point desc_ring is allocated but q_vector
is still NULL, causing a NULL pointer dereference here.

The old code passed dev as a parameter derived from vport->adapter->pdev->dev
which was always valid.

> 	dmam_free_coherent(dev, rxq->size, rxq->desc_ring, rxq->dma);
>  	rxq->desc_ring = NULL;
>  }

> @@ -467,9 +471,10 @@ static void idpf_rx_desc_rel(struct idpf_rx_queue *rxq, struct device *dev,
>   * @bufq: buffer queue to clean the resources from
>   * @dev: device to free DMA memory
>   */
> -static void idpf_rx_desc_rel_bufq(struct idpf_buf_queue *bufq,
> -				  struct device *dev)
> +static void idpf_rx_desc_rel_bufq(struct idpf_buf_queue *bufq)
>  {
> +	struct device *dev;
> +
>  	if (!bufq)
>  		return;
>

[ ... ]

>  	if (!bufq->split_buf)
>  		return;
>
> +	dev = &bufq->q_vector->vport->adapter->pdev->dev;
                 ^^^^^^^^^

The same concern applies here. bufq->q_vector may be NULL if we reach
this cleanup path before idpf_vport_intr_init() has been called.

> 	dma_free_coherent(dev, bufq->size, bufq->split_buf, bufq->dma);
>  	bufq->split_buf = NULL;
>  }