linux-kernel - Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac

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: <20251215120037.000053d7@huawei.com>
Date: Mon, 15 Dec 2025 12:00:37 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>
CC: <dave.jiang@...el.com>, <linux-cxl@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <Smita.KoralahalliChannabasappa@....com>,
	<alison.schofield@...el.com>, <terry.bowman@....com>,
	<alejandro.lucero-palau@....com>, <linux-pci@...r.kernel.org>, Shiju Jose
	<shiju.jose@...wei.com>
Subject: Re: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release()
 confusion

On Wed,  3 Dec 2025 18:21:31 -0800
Dan Williams <dan.j.williams@...el.com> wrote:

> A device release method is only for undoing allocations on the path to
> preparing the device for device_add(). In contrast, devm allocations are
> post device_add(), are acquired during / after ->probe() and are released
> synchronous with ->remove().
> 
> So, a "devm" helper in a "release" method is a clear anti-pattern.
> 
> Move this devm release action where it belongs, an action created at edac
> object creation time. Otherwise, this leaks resources until
> cxl_memdev_release() time which may be long after these xarray and error
> record caches have gone idle.
 
> Note, this also fixes up the type of @cxlmd->err_rec_array which needlessly
> dropped type-safety.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Cc: Dave Jiang <dave.jiang@...el.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Cc: Shiju Jose <shiju.jose@...wei.com>
> Cc: Alison Schofield <alison.schofield@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>

This change seems fine to me. The tear down occurs after unregistering the
edac device and as the repair is via sysfs in there we shouldn't have any
left over calls in flight.  Was an odd code pattern. Oops to missing this
in earlier review.

Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>