netdev - Re: [Intel-wired-lan][PATCH iwl-next v9 01/10] idpf: introduce local idpf structure to store virtchnl queue chunks

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] [day] [month] [year] [list]

Message-ID: <aQ97z8ZZToGIxb3X@horms.kernel.org>
Date: Sat, 8 Nov 2025 17:20:15 +0000
From: Simon Horman <horms@...nel.org>
To: Joshua Hay <joshua.a.hay@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org
Subject: Re: [Intel-wired-lan][PATCH iwl-next v9 01/10] idpf: introduce local
 idpf structure to store virtchnl queue chunks

On Tue, Oct 21, 2025 at 04:30:47PM -0700, Joshua Hay wrote:

...

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c

...

> @@ -1237,6 +1242,8 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>  
>  	return vport;
>  
> +free_qreg_chunks:
> +	kfree(adapter->vport_config[idx]->qid_reg_info.queue_chunks);

I think that the following is also needed here, to avoid a subsequent
double-free.

	adapter->vport_config[idx]->qid_reg_info.queue_chunks = NULL;

>  free_vector_idxs:
>  	kfree(vport->q_vector_idxs);
>  free_vport:

...

> @@ -3658,6 +3668,11 @@ void idpf_vport_init(struct idpf_vport *vport, struct idpf_vport_max_q *max_q)
>  	rss_data = &vport_config->user_config.rss_data;
>  	vport_msg = adapter->vport_params_recvd[idx];
>  
> +	err = idpf_vport_init_queue_reg_chunks(vport_config,
> +					       &vport_msg->chunks);
> +	if (err)
> +		return err;
> +
>  	vport_config->max_q.max_txq = max_q->max_txq;
>  	vport_config->max_q.max_rxq = max_q->max_rxq;
>  	vport_config->max_q.max_complq = max_q->max_complq;
> @@ -3690,15 +3705,17 @@ void idpf_vport_init(struct idpf_vport *vport, struct idpf_vport_max_q *max_q)
>  
>  	if (!(vport_msg->vport_flags &
>  	      cpu_to_le16(VIRTCHNL2_VPORT_UPLINK_PORT)))
> -		return;
> +		return 0;
>  
>  	err = idpf_ptp_get_vport_tstamps_caps(vport);
>  	if (err) {
>  		pci_dbg(vport->adapter->pdev, "Tx timestamping not supported\n");
> -		return;
> +		return err == -EOPNOTSUPP ? 0 : err;

If a non-zero value is returned here, then
the allocation (of adapter->vport_config[idx]->qid_reg_info.queue_chunks)
made in idpf_vport_init_queue_reg_chunks() will be leaked.

I think it should be both freed and set to NULL in this error path.
Which I think suggests a helper to do so here and elsewhere.

Flagged by Claude Code with https://github.com/masoncl/review-prompts/


>  	}
>  
>  	INIT_WORK(&vport->tstamp_task, idpf_tstamp_task);
> +
> +	return 0;
>  }

...