linux-kernel - Re: [PATCH v14 19/34] cxl/port: Fix devm resource leaks around with dport management

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: <20260116161607.00005588@huawei.com>
Date: Fri, 16 Jan 2026 16:16:07 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: <dan.j.williams@...el.com>
CC: Terry Bowman <terry.bowman@....com>, <dave@...olabs.net>,
	<dave.jiang@...el.com>, <alison.schofield@...el.com>, <bhelgaas@...gle.com>,
	<shiju.jose@...wei.com>, <ming.li@...omail.com>,
	<Smita.KoralahalliChannabasappa@....com>, <rrichter@....com>,
	<dan.carpenter@...aro.org>, <PradeepVineshReddy.Kodamati@....com>,
	<lukas@...ner.de>, <Benjamin.Cheatham@....com>,
	<sathyanarayanan.kuppuswamy@...ux.intel.com>, <linux-cxl@...r.kernel.org>,
	<vishal.l.verma@...el.com>, <alucerop@....com>, <ira.weiny@...el.com>,
	<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>
Subject: Re: [PATCH v14 19/34] cxl/port: Fix devm resource leaks around with
 dport management


> 	rc = check_no_existing_dport(port, dport_dev);
> 	if (rc)
> 		return ERR_PTR(rc);
> 
> then can rename new_dport to dport for this patch.
> Or don't bother hiding it and just use dport variable for both. 
> 
> 
> >  	int rc;
> >  
> > @@ -1615,29 +1649,47 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> >  		return ERR_PTR(-EBUSY);
> >  	}
> >  
> > -	struct cxl_dport *new_dport __free(del_cxl_dport) =
> > -		devm_cxl_add_dport_by_dev(port, dport_dev);
> > -	if (IS_ERR(new_dport))
> > -		return new_dport;
> > -
> > -	cxl_switch_parse_cdat(new_dport);
> > +	/*
> > +	 * With the first dport arrival it is now safe to start looking at
> > +	 * component registers. Be careful to not strand resources if dport
> > +	 * creation ultimately fails.
> > +	 */
> > +	struct cxl_port *port_group __free(cxl_port_release_group) =
> > +		cxl_port_open_group(port);  
> 
> So this is relies on everything being registered against port->dev, whereas
> for root ports this then gets handed off to port->uport_dev.
> That's a bit obscure - hence request for some patch description text
> on who owns what resources + when.
> 
> > +	if (IS_ERR(port_group))
> > +		return ERR_CAST(port_group);
> >  
> > -	if (ida_is_empty(&port->decoder_ida)) {
> > +	if (port->nr_dports == 0) {
> >  		rc = devm_cxl_switch_port_decoders_setup(port);
> >  		if (rc)
> >  			return ERR_PTR(rc);
I'm not totally sure I have an appropriate base for messing with this but
with just this patch on top of cxl/for-7.0/cxl-init I'm getting problems
with the reorder here as the devm_cxl_switch_port fails to map HDM decoders.
On a simple single switch couple of devices test.

I'm probably suffering Friday afternoon syndrome (and naughty testing just
a middle patch in a series)

My 'guess' is that it's because we no longer add the port first.
I couldn't spot any other patch messing with related logic earlier in Terry's series
(and wanted to keep what I was messing with for trying alternative fixes to a minimum)

Any thoughts on what I'm missing?

Jonathan


> > -		dev_dbg(&port->dev, "first dport%d:%s added with decoders\n",
> > -			new_dport->port_id, dev_name(dport_dev));
> > -		return no_free_ptr(new_dport);
> > +		/*
> > +		 * Note, when nr_dports returns to zero the port is unregistered
> > +		 * and triggers cleanup. I.e. no need for open-coded release
> > +		 * action on dport removal. See cxl_detach_ep() for that logic.
> > +		 */
> >  	}
> >  
> > +	new_dport = cxl_add_dport_by_dev(port, dport_dev);
> > +	if (IS_ERR(new_dport))
> > +		return new_dport;
> > +
> > +	rc = cxl_dport_autoremove(new_dport);
> > +	if (rc)
> > +		return ERR_PTR(rc);
> > +
> > +	cxl_switch_parse_cdat(new_dport);
> > +
> > +	/* group tracking no longer needed, dport successfully added */
> > +	cxl_port_remove_group(no_free_ptr(port_group));