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: <DM4PR11MB650291959A827408C0AEE2B2D4CCA@DM4PR11MB6502.namprd11.prod.outlook.com>
Date: Wed, 12 Nov 2025 18:49:48 +0000
From: "Hay, Joshua A" <joshua.a.hay@...el.com>
To: Simon Horman <horms@...nel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.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

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

Thanks for catching this, will send out a v10 with the fixes soon.

-Josh

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