| 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: <20251204022136.2573521-2-dan.j.williams@intel.com>
Date: Wed, 3 Dec 2025 18:21:31 -0800
From: Dan Williams <dan.j.williams@...el.com>
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@...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>
---
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