netdev - Re: [PATCH v16 02/22] sfc: add cxl support

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: <682e09813a374_1626e100e@dwillia2-xfh.jf.intel.com.notmuch>
Date: Wed, 21 May 2025 10:12:33 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Alejandro Lucero Palau <alucerop@....com>, <dan.j.williams@...el.com>,
	<alejandro.lucero-palau@....com>, <linux-cxl@...r.kernel.org>,
	<netdev@...r.kernel.org>, <edward.cree@....com>, <davem@...emloft.net>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<dave.jiang@...el.com>
CC: Jonathan Cameron <Jonathan.Cameron@...wei.com>, Edward Cree
	<ecree.xilinx@...il.com>
Subject: Re: [PATCH v16 02/22] sfc: add cxl support

Alejandro Lucero Palau wrote:
[..]
> >> +void efx_cxl_exit(struct efx_probe_data *probe_data)
> >> +{
> > So this is empty which means it leaks the cxl_dev_state_create()
> > allocation, right?
> 
> 
> Yes, because I was wrongly relying on devres ...
> 
> 
> Previous patchsets were doing the explicit release here.
> 
> 
> Your suggestion below relies on adding more awareness of cxl into 
> generic efx code, what we want to avoid using the specific efx_cxl.* files.
> 
> As I mentioned in patch 1, I think the right thing to do is to add 
> devres for cxl_dev_state_create.

...but I thought netdev is anti-devres? I am ok having a
devm_cxl_dev_state_create() alongside a "manual" cxl_dev_state_create()
if that is the case.

> Before sending v17 with this change, are you ok with the rest of the 
> patches or you want to go through them as well?

So I did start taking a look and then turned away upon finding a
memory-leak on the first 2 patches in the series. I will continue going
through it, but in general the lifetime and locking rules of the CXL
subsystem continue to be a source of trouble in new enabling. At a
minimum that indicates a need/opportunity to review the rules at a
future CXL collab meeting.