| 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
| ||
|
Message-ID: <4a489f66af1c4f83b0ece5d54ef35fc9@huawei.com>
Date: Mon, 8 Dec 2025 19:20:15 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>, "dave.jiang@...el.com"
<dave.jiang@...el.com>
CC: "linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Smita.KoralahalliChannabasappa@....com"
<Smita.KoralahalliChannabasappa@....com>, "alison.schofield@...el.com"
<alison.schofield@...el.com>, "terry.bowman@....com" <terry.bowman@....com>,
"alejandro.lucero-palau@....com" <alejandro.lucero-palau@....com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>, Jonathan Cameron
<jonathan.cameron@...wei.com>
Subject: RE: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
>-----Original Message-----
>From: Dan Williams <dan.j.williams@...el.com>
>Sent: 04 December 2025 02:22
>To: dave.jiang@...el.com
>Cc: 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; Jonathan Cameron <jonathan.cameron@...wei.com>;
>Shiju Jose <shiju.jose@...wei.com>
>Subject: [PATCH 1/6] cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
>
>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>
Tested-by: Shiju Jose <shiju.jose@...wei.com>
Reviewed-by: Shiju Jose <shiju.jose@...wei.com>
>---
> drivers/cxl/cxlmem.h | 5 +--
> drivers/cxl/core/edac.c | 64 ++++++++++++++++++++++-----------------
> drivers/cxl/core/memdev.c | 1 -
> 3 files changed, 38 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index
>434031a0c1f7..c12ab4fc9512 100644
>--- a/drivers/cxl/cxlmem.h
>+++ b/drivers/cxl/cxlmem.h
>@@ -63,7 +63,7 @@ struct cxl_memdev {
> int depth;
> u8 scrub_cycle;
> int scrub_region_id;
>- void *err_rec_array;
>+ struct cxl_mem_err_rec *err_rec_array;
> };
>
> static inline struct cxl_memdev *to_cxl_memdev(struct device *dev) @@ -877,7
>+877,6 @@ int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd);
>int devm_cxl_region_edac_register(struct cxl_region *cxlr); int
>cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt); int
>cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt); -void
>devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd); #else static
>inline int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd) { return
>0; } @@ -889,8 +888,6 @@ static inline int cxl_store_rec_gen_media(struct
>cxl_memdev *cxlmd, static inline int cxl_store_rec_dram(struct cxl_memdev
>*cxlmd,
> union cxl_event *evt)
> { return 0; }
>-static inline void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd) -
>{ return; } #endif
>
> #ifdef CONFIG_CXL_SUSPEND
>diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c index
>79994ca9bc9f..81160260e26b 100644
>--- a/drivers/cxl/core/edac.c
>+++ b/drivers/cxl/core/edac.c
>@@ -1988,6 +1988,40 @@ static int cxl_memdev_soft_ppr_init(struct
>cxl_memdev *cxlmd,
> return 0;
> }
>
>+static void err_rec_free(void *_cxlmd)
>+{
>+ struct cxl_memdev *cxlmd = _cxlmd;
>+ struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
>+ struct cxl_event_gen_media *rec_gen_media;
>+ struct cxl_event_dram *rec_dram;
>+ unsigned long index;
>+
>+ cxlmd->err_rec_array = NULL;
>+ xa_for_each(&array_rec->rec_dram, index, rec_dram)
>+ kfree(rec_dram);
>+ xa_destroy(&array_rec->rec_dram);
>+
>+ xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
>+ kfree(rec_gen_media);
>+ xa_destroy(&array_rec->rec_gen_media);
>+ kfree(array_rec);
>+}
>+
>+static int devm_cxl_memdev_setup_err_rec(struct cxl_memdev *cxlmd) {
>+ struct cxl_mem_err_rec *array_rec =
>+ kzalloc(sizeof(*array_rec), GFP_KERNEL);
>+
>+ if (!array_rec)
>+ return -ENOMEM;
>+
>+ xa_init(&array_rec->rec_gen_media);
>+ xa_init(&array_rec->rec_dram);
>+ cxlmd->err_rec_array = array_rec;
>+
>+ return devm_add_action_or_reset(&cxlmd->dev, err_rec_free, cxlmd); }
>+
> int devm_cxl_memdev_edac_register(struct cxl_memdev *cxlmd) {
> struct edac_dev_feature ras_features[CXL_NR_EDAC_DEV_FEATURES];
>@@ -2038,15 +2072,9 @@ int devm_cxl_memdev_edac_register(struct
>cxl_memdev *cxlmd)
> }
>
> if (repair_inst) {
>- struct cxl_mem_err_rec *array_rec =
>- devm_kzalloc(&cxlmd->dev, sizeof(*array_rec),
>- GFP_KERNEL);
>- if (!array_rec)
>- return -ENOMEM;
>-
>- xa_init(&array_rec->rec_gen_media);
>- xa_init(&array_rec->rec_dram);
>- cxlmd->err_rec_array = array_rec;
>+ rc = devm_cxl_memdev_setup_err_rec(cxlmd);
>+ if (rc)
>+ return rc;
> }
> }
>
>@@ -2088,22 +2116,4 @@ int devm_cxl_region_edac_register(struct
>cxl_region *cxlr) } EXPORT_SYMBOL_NS_GPL(devm_cxl_region_edac_register,
>"CXL");
>
>-void devm_cxl_memdev_edac_release(struct cxl_memdev *cxlmd) -{
>- struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
>- struct cxl_event_gen_media *rec_gen_media;
>- struct cxl_event_dram *rec_dram;
>- unsigned long index;
>-
>- if (!IS_ENABLED(CONFIG_CXL_EDAC_MEM_REPAIR) || !array_rec)
>- return;
>-
>- xa_for_each(&array_rec->rec_dram, index, rec_dram)
>- kfree(rec_dram);
>- xa_destroy(&array_rec->rec_dram);
>
>- xa_for_each(&array_rec->rec_gen_media, index, rec_gen_media)
>- kfree(rec_gen_media);
>- xa_destroy(&array_rec->rec_gen_media);
>-}
>-EXPORT_SYMBOL_NS_GPL(devm_cxl_memdev_edac_release, "CXL"); diff --git
>a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index
>e370d733e440..4dff7f44d908 100644
>--- a/drivers/cxl/core/memdev.c
>+++ b/drivers/cxl/core/memdev.c
>@@ -27,7 +27,6 @@ static void cxl_memdev_release(struct device *dev)
> struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>
> ida_free(&cxl_memdev_ida, cxlmd->id);
>- devm_cxl_memdev_edac_release(cxlmd);
> kfree(cxlmd);
> }
>
>--
>2.51.1
>
Powered by blists - more mailing lists