CXL testing environment can trigger following trace
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
Call Trace:
<TASK>
cxl_event_trace_record+0xd1/0xa70 [cxl_core]
__cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
cxl_mem_get_records_log+0x261/0x500 [cxl_core]
cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
platform_probe+0x9d/0x130
really_probe+0x1c8/0x960
__driver_probe_device+0x187/0x3e0
driver_probe_device+0x45/0x120
__device_attach_driver+0x15d/0x280
commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
valid endpoint.
Across the CXL core, endpoint availability is generally determined by
checking whether it is NULL. Align with this convention by initializing
cxlmd->endpoint to NULL by default.
Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
drivers/cxl/core/memdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index af3d0cc65138..41a507b5daa4 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
cxlmd->id = rc;
cxlmd->depth = -1;
cxlmd->attach = attach;
- cxlmd->endpoint = ERR_PTR(-ENXIO);
+ cxlmd->endpoint = NULL;
dev = &cxlmd->dev;
device_initialize(dev);
--
2.43.0
Li Ming wrote:
> CXL testing environment can trigger following trace
>
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> Call Trace:
> <TASK>
> cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
Hooray for unit tests, but I wonder why this failure is so reliable for
you and absent for me? I retried with latest cxl/next, no luck.
No matter, it still looks like something worth addressing, but not with
setting ->endpoint to NULL.
> platform_probe+0x9d/0x130
> really_probe+0x1c8/0x960
> __driver_probe_device+0x187/0x3e0
> driver_probe_device+0x45/0x120
> __device_attach_driver+0x15d/0x280
>
> commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> valid endpoint.
>
> Across the CXL core, endpoint availability is generally determined by
> checking whether it is NULL. Align with this convention by initializing
> cxlmd->endpoint to NULL by default.
>
> Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
> drivers/cxl/core/memdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index af3d0cc65138..41a507b5daa4 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> cxlmd->id = rc;
> cxlmd->depth = -1;
> cxlmd->attach = attach;
> - cxlmd->endpoint = ERR_PTR(-ENXIO);
> + cxlmd->endpoint = NULL;
So, this does not look like a fix to me, it looks like a bug report
against all the code paths that want to assume that a mere NULL check of
->endpoint is sufficient.
I think this unintentional ERR_PTR() trip hazard is now a *feature* in
retrospect to catch those cases. If something depends on ->endpoint
being valid, it depends on ->endpoint *staying* valid.
A proposed fix for this case is below (passes cxl_test), and if other
ERR_PTR() crashes show up I expect they need similar inspection. This
probably wants additional cleanup to consolidate boiler-plate and make
the critical change to cxl_dpa_to_region() easier to see:
-- 8< --
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 422531799af2..e652980098cf 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -39,7 +39,7 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
int cxl_region_init(void);
void cxl_region_exit(void);
int cxl_get_poison_by_endpoint(struct cxl_port *port);
-struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
+struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa);
u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
u64 dpa);
@@ -50,7 +50,7 @@ static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
return ULLONG_MAX;
}
static inline
-struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
+struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
{
return NULL;
}
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ef202b34e5ea..610ed6744ddc 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -864,7 +864,7 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
unsigned long *cmds);
void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
-void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
+void cxl_event_trace_record(struct cxl_memdev *cxlmd,
enum cxl_event_log_type type,
enum cxl_event_type event_type,
const uuid_t *uuid, union cxl_event *evt);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0be95294b6e6..4fafee80524b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -911,6 +911,7 @@ static inline void device_unlock(struct device *dev)
}
DEFINE_GUARD(device, struct device *, device_lock(_T), device_unlock(_T))
+DEFINE_GUARD_COND(device, _intr, device_lock_interruptible(_T), _RET == 0)
static inline void device_lock_assert(struct device *dev)
{
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index fa6dd0c94656..7b923408b6c5 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -886,7 +886,7 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
}
EXPORT_SYMBOL_NS_GPL(cxl_enumerate_cmds, "CXL");
-void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
+void cxl_event_trace_record(struct cxl_memdev *cxlmd,
enum cxl_event_log_type type,
enum cxl_event_type event_type,
const uuid_t *uuid, union cxl_event *evt)
@@ -913,6 +913,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
* translations. Take topology mutation locks and lookup
* { HPA, REGION } from { DPA, MEMDEV } in the event record.
*/
+ guard(device)(&cxlmd->dev);
guard(rwsem_read)(&cxl_rwsem.region);
guard(rwsem_read)(&cxl_rwsem.dpa);
@@ -961,7 +962,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
}
EXPORT_SYMBOL_NS_GPL(cxl_event_trace_record, "CXL");
-static void __cxl_event_trace_record(const struct cxl_memdev *cxlmd,
+static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
enum cxl_event_log_type type,
struct cxl_event_record_raw *record)
{
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index af3d0cc65138..d2bee5608e50 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -331,6 +331,10 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
{
int rc;
+ ACQUIRE(device_intr, devlock)(&cxlmd->dev);
+ if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+ return rc;
+
ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
return rc;
@@ -355,6 +359,7 @@ int cxl_clear_poison_locked(struct cxl_memdev *cxlmd, u64 dpa)
if (!IS_ENABLED(CONFIG_DEBUG_FS))
return 0;
+ device_lock_assert(&cxlmd->dev);
lockdep_assert_held(&cxl_rwsem.dpa);
lockdep_assert_held(&cxl_rwsem.region);
@@ -400,6 +405,10 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
{
int rc;
+ ACQUIRE(device_intr, devlock)(&cxlmd->dev);
+ if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+ return rc;
+
ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
return rc;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 96888d87a8df..a7d391757e16 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2936,7 +2936,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
return 1;
}
-struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
+struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
{
struct cxl_dpa_to_region_context ctx;
struct cxl_port *port;
@@ -2944,8 +2944,12 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
ctx = (struct cxl_dpa_to_region_context) {
.dpa = dpa,
};
+
+ device_lock_assert(&cxlmd->dev);
+ if (!cxlmd->dev.driver)
+ return NULL;
port = cxlmd->endpoint;
- if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
+ if (cxl_num_decoders_committed(port))
device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
return ctx.cxlr;
@@ -4004,10 +4008,9 @@ static int validate_region_offset(struct cxl_region *cxlr, u64 offset)
return 0;
}
-static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
+static int region_poison_lookup(struct cxl_region *cxlr, u64 offset,
+ struct dpa_result *result)
{
- struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
- struct cxl_region *cxlr = data;
int rc;
ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
@@ -4022,8 +4025,8 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
return -EINVAL;
offset -= cxlr->params.cache_size;
- rc = region_offset_to_dpa_result(cxlr, offset, &result);
- if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
+ rc = region_offset_to_dpa_result(cxlr, offset, result);
+ if (rc || !result->cxlmd || result->dpa == ULLONG_MAX) {
dev_dbg(&cxlr->dev,
"Failed to resolve DPA for region offset %#llx rc %d\n",
offset, rc);
@@ -4031,7 +4034,43 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
return rc ? rc : -EINVAL;
}
- return cxl_inject_poison_locked(result.cxlmd, result.dpa);
+ return 0;
+}
+
+static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
+{
+ struct dpa_result result1 = { .dpa = ULLONG_MAX };
+ struct dpa_result result2 = { .dpa = ULLONG_MAX };
+ struct cxl_region *cxlr = data;
+ struct cxl_memdev *cxlmd;
+ int rc;
+
+ rc = region_poison_lookup(cxlr, offset, &result1);
+ if (rc)
+ return rc;
+
+ cxlmd = result1.cxlmd;
+ ACQUIRE(device_intr, devlock)(&cxlmd->dev);
+ if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+ return rc;
+
+ ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
+ if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
+ return rc;
+
+ ACQUIRE(rwsem_read_intr, dpa_rwsem)(&cxl_rwsem.dpa);
+ if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
+ return rc;
+
+ offset -= cxlr->params.cache_size;
+ rc = region_offset_to_dpa_result(cxlr, offset, &result2);
+ if (rc || memcmp(&result1, &result2, sizeof(result1) != 0)) {
+ dev_dbg(&cxlr->dev,
+ "Error injection raced region reconfiguration: %d\n", rc);
+ return rc ? rc : -ENXIO;
+ }
+
+ return cxl_inject_poison_locked(result1.cxlmd, result1.dpa);
}
DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
@@ -4039,10 +4078,21 @@ DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
{
- struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
+ struct dpa_result result1 = { .dpa = ULLONG_MAX };
+ struct dpa_result result2 = { .dpa = ULLONG_MAX };
struct cxl_region *cxlr = data;
+ struct cxl_memdev *cxlmd;
int rc;
+ rc = region_poison_lookup(cxlr, offset, &result1);
+ if (rc)
+ return rc;
+
+ cxlmd = result1.cxlmd;
+ ACQUIRE(device_intr, devlock)(&cxlmd->dev);
+ if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
+ return rc;
+
ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
return rc;
@@ -4051,20 +4101,15 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
return rc;
- if (validate_region_offset(cxlr, offset))
- return -EINVAL;
-
offset -= cxlr->params.cache_size;
- rc = region_offset_to_dpa_result(cxlr, offset, &result);
- if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
+ rc = region_offset_to_dpa_result(cxlr, offset, &result2);
+ if (rc || memcmp(&result1, &result2, sizeof(result1) != 0)) {
dev_dbg(&cxlr->dev,
- "Failed to resolve DPA for region offset %#llx rc %d\n",
- offset, rc);
-
- return rc ? rc : -EINVAL;
+ "Error clearing raced region reconfiguration: %d\n", rc);
+ return rc ? rc : -ENXIO;
}
- return cxl_clear_poison_locked(result.cxlmd, result.dpa);
+ return cxl_clear_poison_locked(result1.cxlmd, result1.dpa);
}
DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
--
2.52.0
From: <dan.j.williams@intel.com>
To: "Li Ming"<ming.li@zohomail.com>, <dave@stgolabs.net>, <jonathan.cameron@huawei.com>, <dave.jiang@intel.com>, <alison.schofield@intel.com>, <vishal.l.verma@intel.com>, <ira.weiny@intel.com>, <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>, "Li Ming"<ming.li@zohomail.com>
Date: Tue, 03 Feb 2026 08:01:04 +0800
Subject: Re: [PATCH 1/2] cxl/core: Set cxlmd->endpoint to NULL by default
> Li Ming wrote:
> > CXL testing environment can trigger following trace
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> > KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> > RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> > Call Trace:
> > <TASK>
> > cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> > __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> > cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> > cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> > cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
>
> Hooray for unit tests, but I wonder why this failure is so reliable for
> you and absent for me? I retried with latest cxl/next, no luck.
>
> No matter, it still looks like something worth addressing, but not with
> setting ->endpoint to NULL.
>
> > platform_probe+0x9d/0x130
> > really_probe+0x1c8/0x960
> > __driver_probe_device+0x187/0x3e0
> > driver_probe_device+0x45/0x120
> > __device_attach_driver+0x15d/0x280
> >
> > commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> > However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> > valid endpoint.
> >
> > Across the CXL core, endpoint availability is generally determined by
> > checking whether it is NULL. Align with this convention by initializing
> > cxlmd->endpoint to NULL by default.
> >
> > Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > Signed-off-by: Li Ming <ming.li@zohomail.com>
> > ---
> > drivers/cxl/core/memdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index af3d0cc65138..41a507b5daa4 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> > cxlmd->id = rc;
> > cxlmd->depth = -1;
> > cxlmd->attach = attach;
> > - cxlmd->endpoint = ERR_PTR(-ENXIO);
> > + cxlmd->endpoint = NULL;
>
> So, this does not look like a fix to me, it looks like a bug report
> against all the code paths that want to assume that a mere NULL check of
> ->endpoint is sufficient.
>
> I think this unintentional ERR_PTR() trip hazard is now a *feature* in
> retrospect to catch those cases. If something depends on ->endpoint
> being valid, it depends on ->endpoint *staying* valid.
>
> A proposed fix for this case is below (passes cxl_test), and if other
> ERR_PTR() crashes show up I expect they need similar inspection. This
> probably wants additional cleanup to consolidate boiler-plate and make
> the critical change to cxl_dpa_to_region() easier to see:
>
Hi Dan,
Thanks for your proposal, I think your change can solve this problem too.
But the change is a lot, and need more time to review all driver code to confirm
if there are other places needed such checking. (I found that cxl_reset_done also needs some changes like you mentioned)
Maybe we can consider my change as a quick fix? Then I can prepare a new patchset for the consolidation.
Ming
> -- 8< --
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 422531799af2..e652980098cf 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -39,7 +39,7 @@ int cxl_decoder_detach(struct cxl_region *cxlr,
> int cxl_region_init(void);
> void cxl_region_exit(void);
> int cxl_get_poison_by_endpoint(struct cxl_port *port);
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa);
> u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> u64 dpa);
>
> @@ -50,7 +50,7 @@ static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
> return ULLONG_MAX;
> }
> static inline
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
> {
> return NULL;
> }
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ef202b34e5ea..610ed6744ddc 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -864,7 +864,7 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> unsigned long *cmds);
> void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
> -void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> +void cxl_event_trace_record(struct cxl_memdev *cxlmd,
> enum cxl_event_log_type type,
> enum cxl_event_type event_type,
> const uuid_t *uuid, union cxl_event *evt);
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..4fafee80524b 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -911,6 +911,7 @@ static inline void device_unlock(struct device *dev)
> }
>
> DEFINE_GUARD(device, struct device *, device_lock(_T), device_unlock(_T))
> +DEFINE_GUARD_COND(device, _intr, device_lock_interruptible(_T), _RET == 0)
>
> static inline void device_lock_assert(struct device *dev)
> {
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index fa6dd0c94656..7b923408b6c5 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -886,7 +886,7 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_enumerate_cmds, "CXL");
>
> -void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> +void cxl_event_trace_record(struct cxl_memdev *cxlmd,
> enum cxl_event_log_type type,
> enum cxl_event_type event_type,
> const uuid_t *uuid, union cxl_event *evt)
> @@ -913,6 +913,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> * translations. Take topology mutation locks and lookup
> * { HPA, REGION } from { DPA, MEMDEV } in the event record.
> */
> + guard(device)(&cxlmd->dev);
> guard(rwsem_read)(&cxl_rwsem.region);
> guard(rwsem_read)(&cxl_rwsem.dpa);
>
> @@ -961,7 +962,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> }
> EXPORT_SYMBOL_NS_GPL(cxl_event_trace_record, "CXL");
>
> -static void __cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> +static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
> enum cxl_event_log_type type,
> struct cxl_event_record_raw *record)
> {
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index af3d0cc65138..d2bee5608e50 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -331,6 +331,10 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
> {
> int rc;
>
> + ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> + if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> + return rc;
> +
> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
> if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
> return rc;
> @@ -355,6 +359,7 @@ int cxl_clear_poison_locked(struct cxl_memdev *cxlmd, u64 dpa)
> if (!IS_ENABLED(CONFIG_DEBUG_FS))
> return 0;
>
> + device_lock_assert(&cxlmd->dev);
> lockdep_assert_held(&cxl_rwsem.dpa);
> lockdep_assert_held(&cxl_rwsem.region);
>
> @@ -400,6 +405,10 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
> {
> int rc;
>
> + ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> + if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> + return rc;
> +
> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
> if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
> return rc;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 96888d87a8df..a7d391757e16 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2936,7 +2936,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
> return 1;
> }
>
> -struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> +struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
> {
> struct cxl_dpa_to_region_context ctx;
> struct cxl_port *port;
> @@ -2944,8 +2944,12 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> ctx = (struct cxl_dpa_to_region_context) {
> .dpa = dpa,
> };
> +
> + device_lock_assert(&cxlmd->dev);
> + if (!cxlmd->dev.driver)
> + return NULL;
> port = cxlmd->endpoint;
> - if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
> + if (cxl_num_decoders_committed(port))
> device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
>
> return ctx.cxlr;
> @@ -4004,10 +4008,9 @@ static int validate_region_offset(struct cxl_region *cxlr, u64 offset)
> return 0;
> }
>
> -static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
> +static int region_poison_lookup(struct cxl_region *cxlr, u64 offset,
> + struct dpa_result *result)
> {
> - struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
> - struct cxl_region *cxlr = data;
> int rc;
>
> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
> @@ -4022,8 +4025,8 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
> return -EINVAL;
>
> offset -= cxlr->params.cache_size;
> - rc = region_offset_to_dpa_result(cxlr, offset, &result);
> - if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
> + rc = region_offset_to_dpa_result(cxlr, offset, result);
> + if (rc || !result->cxlmd || result->dpa == ULLONG_MAX) {
> dev_dbg(&cxlr->dev,
> "Failed to resolve DPA for region offset %#llx rc %d\n",
> offset, rc);
> @@ -4031,7 +4034,43 @@ static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
> return rc ? rc : -EINVAL;
> }
>
> - return cxl_inject_poison_locked(result.cxlmd, result.dpa);
> + return 0;
> +}
> +
> +static int cxl_region_debugfs_poison_inject(void *data, u64 offset)
> +{
> + struct dpa_result result1 = { .dpa = ULLONG_MAX };
> + struct dpa_result result2 = { .dpa = ULLONG_MAX };
> + struct cxl_region *cxlr = data;
> + struct cxl_memdev *cxlmd;
> + int rc;
> +
> + rc = region_poison_lookup(cxlr, offset, &result1);
> + if (rc)
> + return rc;
> +
> + cxlmd = result1.cxlmd;
> + ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> + if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> + return rc;
> +
> + ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
> + return rc;
> +
> + ACQUIRE(rwsem_read_intr, dpa_rwsem)(&cxl_rwsem.dpa);
> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
> + return rc;
> +
> + offset -= cxlr->params.cache_size;
> + rc = region_offset_to_dpa_result(cxlr, offset, &result2);
> + if (rc || memcmp(&result1, &result2, sizeof(result1) != 0)) {
> + dev_dbg(&cxlr->dev,
> + "Error injection raced region reconfiguration: %d\n", rc);
> + return rc ? rc : -ENXIO;
> + }
> +
> + return cxl_inject_poison_locked(result1.cxlmd, result1.dpa);
> }
>
> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
> @@ -4039,10 +4078,21 @@ DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
>
> static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
> {
> - struct dpa_result result = { .dpa = ULLONG_MAX, .cxlmd = NULL };
> + struct dpa_result result1 = { .dpa = ULLONG_MAX };
> + struct dpa_result result2 = { .dpa = ULLONG_MAX };
> struct cxl_region *cxlr = data;
> + struct cxl_memdev *cxlmd;
> int rc;
>
> + rc = region_poison_lookup(cxlr, offset, &result1);
> + if (rc)
> + return rc;
> +
> + cxlmd = result1.cxlmd;
> + ACQUIRE(device_intr, devlock)(&cxlmd->dev);
> + if ((rc = ACQUIRE_ERR(device_intr, &devlock)))
> + return rc;
> +
> ACQUIRE(rwsem_read_intr, region_rwsem)(&cxl_rwsem.region);
> if ((rc = ACQUIRE_ERR(rwsem_read_intr, ®ion_rwsem)))
> return rc;
> @@ -4051,20 +4101,15 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
> if ((rc = ACQUIRE_ERR(rwsem_read_intr, &dpa_rwsem)))
> return rc;
>
> - if (validate_region_offset(cxlr, offset))
> - return -EINVAL;
> -
> offset -= cxlr->params.cache_size;
> - rc = region_offset_to_dpa_result(cxlr, offset, &result);
> - if (rc || !result.cxlmd || result.dpa == ULLONG_MAX) {
> + rc = region_offset_to_dpa_result(cxlr, offset, &result2);
> + if (rc || memcmp(&result1, &result2, sizeof(result1) != 0)) {
> dev_dbg(&cxlr->dev,
> - "Failed to resolve DPA for region offset %#llx rc %d\n",
> - offset, rc);
> -
> - return rc ? rc : -EINVAL;
> + "Error clearing raced region reconfiguration: %d\n", rc);
> + return rc ? rc : -ENXIO;
> }
>
> - return cxl_clear_poison_locked(result.cxlmd, result.dpa);
> + return cxl_clear_poison_locked(result1.cxlmd, result1.dpa);
> }
>
> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
> --
> 2.52.0
>
>
Li Ming wrote: [..] > Hi Dan, > > Thanks for your proposal, I think your change can solve this problem too. > But the change is a lot, and need more time to review all driver code > to confirm if there are other places needed such checking. (I found > that cxl_reset_done also needs some changes like you mentioned) Maybe > we can consider my change as a quick fix? Then I can prepare a new > patchset for the consolidation. I am not convinced that it is a fix. The fact that you say the bug disappears when patch2 is applied leads me to believe that is potentially the only bug. I.e. it may be the case that cxl_dpa_to_region() is safe to assume that a valid ->endpoint pointer will remain valid once the port bus_add_device() vs bus_probe_device() hole is plugged. I would say start with patch2 by itself. Then circle back to prove that a mere NULL check is a fix or just makes the vulnerability window smaller and the locking rework is needed.
On 2/1/26 2:30 AM, Li Ming wrote:
> CXL testing environment can trigger following trace
>
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> Call Trace:
> <TASK>
> cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
> platform_probe+0x9d/0x130
> really_probe+0x1c8/0x960
> __driver_probe_device+0x187/0x3e0
> driver_probe_device+0x45/0x120
> __device_attach_driver+0x15d/0x280
So I talked to Dan and the assigning of ERR_PTR(-ENXIO) is from a previous revision of implementation and isn't intended to be there. So removing the line entirely (which would mean endpoint == NULL from kzalloc) is probably the right change. However, I'm not able to reproduce the error. How is it triggered? Also, by setting it to NULL, are we covering up a different issue? Are you able to determine whether cxlmd->endpoint at that point is unexpected and perhaps triggered by a different bug?
DJ
>
> commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> valid endpoint.
>
> Across the CXL core, endpoint availability is generally determined by
> checking whether it is NULL. Align with this convention by initializing
> cxlmd->endpoint to NULL by default.
>
> Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
> drivers/cxl/core/memdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index af3d0cc65138..41a507b5daa4 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> cxlmd->id = rc;
> cxlmd->depth = -1;
> cxlmd->attach = attach;
> - cxlmd->endpoint = ERR_PTR(-ENXIO);
> + cxlmd->endpoint = NULL;
>
> dev = &cxlmd->dev;
> device_initialize(dev);
From: Dave Jiang <dave.jiang@intel.com>
To: "Li Ming"<ming.li@zohomail.com>, <dave@stgolabs.net>, <jonathan.cameron@huawei.com>, <alison.schofield@intel.com>, <vishal.l.verma@intel.com>, <ira.weiny@intel.com>, <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Date: Tue, 03 Feb 2026 05:04:44 +0800
Subject: Re: [PATCH 1/2] cxl/core: Set cxlmd->endpoint to NULL by default
>
>
> On 2/1/26 2:30 AM, Li Ming wrote:
> > CXL testing environment can trigger following trace
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> > KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> > RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> > Call Trace:
> > <TASK>
> > cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> > __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> > cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> > cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> > cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
> > platform_probe+0x9d/0x130
> > really_probe+0x1c8/0x960
> > __driver_probe_device+0x187/0x3e0
> > driver_probe_device+0x45/0x120
> > __device_attach_driver+0x15d/0x280
>
> So I talked to Dan and the assigning of ERR_PTR(-ENXIO) is from a previous revision of implementation and isn't intended to be there. So removing the line entirely (which would mean endpoint == NULL from kzalloc) is probably the right change. However, I'm not able to reproduce the error. How is it triggered? Also, by setting it to NULL, are we covering up a different issue? Are you able to determine whether cxlmd->endpoint at that point is unexpected and perhaps triggered by a different bug?
>
> DJ
Hi Dave,
As I implied in cover-letter, When I trigger issue #2, it always trigger this issue.
I have a simple script to run cxl test over and over again in a VM. I usually reproduce it in one hour with KASAN enabled.
When I reproduced it, I dumpped out the value of cxlmd->endpoint, it is -ENXIO. That means it was not updated in cxl_port_add() yet at that time. I cannot reproduce this issue only with patch #2.
Ming
>
> >
> > commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> > However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> > valid endpoint.
> >
> > Across the CXL core, endpoint availability is generally determined by
> > checking whether it is NULL. Align with this convention by initializing
> > cxlmd->endpoint to NULL by default.
> >
> > Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > Signed-off-by: Li Ming <ming.li@zohomail.com>
> > ---
> > drivers/cxl/core/memdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index af3d0cc65138..41a507b5daa4 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> > cxlmd->id = rc;
> > cxlmd->depth = -1;
> > cxlmd->attach = attach;
> > - cxlmd->endpoint = ERR_PTR(-ENXIO);
> > + cxlmd->endpoint = NULL;
> >
> > dev = &cxlmd->dev;
> > device_initialize(dev);
>
>
On Sun, 1 Feb 2026 17:30:01 +0800
Li Ming <ming.li@zohomail.com> wrote:
> CXL testing environment can trigger following trace
>
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> Call Trace:
> <TASK>
> cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
> platform_probe+0x9d/0x130
> really_probe+0x1c8/0x960
> __driver_probe_device+0x187/0x3e0
> driver_probe_device+0x45/0x120
> __device_attach_driver+0x15d/0x280
>
> commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> valid endpoint.
>
> Across the CXL core, endpoint availability is generally determined by
> checking whether it is NULL. Align with this convention by initializing
> cxlmd->endpoint to NULL by default.
I had a look at whether it made sense to use use IS_ERR_OR_NULL() to check
for validity of the endpoint, but it would be somewhat fiddly and I think
you are correct that convention here seems to be NULL means not set.
We don't need the error code. One comment inline.
Either way nice catch
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
> Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
> drivers/cxl/core/memdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index af3d0cc65138..41a507b5daa4 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> cxlmd->id = rc;
> cxlmd->depth = -1;
> cxlmd->attach = attach;
> - cxlmd->endpoint = ERR_PTR(-ENXIO);
> + cxlmd->endpoint = NULL;
cxlmd has just been allocated with kzalloc so I'd argue we don't need this to be explicitly
set at all. Seems like a natural and safe default.
>
> dev = &cxlmd->dev;
> device_initialize(dev);
From: Jonathan Cameron <jonathan.cameron@huawei.com>
To: "Li Ming"<ming.li@zohomail.com>
Cc: <dave@stgolabs.net>, <dave.jiang@intel.com>, <alison.schofield@intel.com>, <vishal.l.verma@intel.com>, <ira.weiny@intel.com>, <dan.j.williams@intel.com>, <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Date: Mon, 02 Feb 2026 22:41:03 +0800
Subject: Re: [PATCH 1/2] cxl/core: Set cxlmd->endpoint to NULL by default
> On Sun, 1 Feb 2026 17:30:01 +0800
> Li Ming <ming.li@zohomail.com> wrote:
>
> > CXL testing environment can trigger following trace
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> > KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> > RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> > Call Trace:
> > <TASK>
> > cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> > __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> > cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> > cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> > cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
> > platform_probe+0x9d/0x130
> > really_probe+0x1c8/0x960
> > __driver_probe_device+0x187/0x3e0
> > driver_probe_device+0x45/0x120
> > __device_attach_driver+0x15d/0x280
> >
> > commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> > However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> > valid endpoint.
> >
> > Across the CXL core, endpoint availability is generally determined by
> > checking whether it is NULL. Align with this convention by initializing
> > cxlmd->endpoint to NULL by default.
>
> I had a look at whether it made sense to use use IS_ERR_OR_NULL() to check
> for validity of the endpoint, but it would be somewhat fiddly and I think
> you are correct that convention here seems to be NULL means not set.
> We don't need the error code. One comment inline.
>
> Either way nice catch
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Thanks for the review.
>
>
> >
> > Fixes: 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > Signed-off-by: Li Ming <ming.li@zohomail.com>
> > ---
> > drivers/cxl/core/memdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index af3d0cc65138..41a507b5daa4 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -675,7 +675,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
> > cxlmd->id = rc;
> > cxlmd->depth = -1;
> > cxlmd->attach = attach;
> > - cxlmd->endpoint = ERR_PTR(-ENXIO);
> > + cxlmd->endpoint = NULL;
> cxlmd has just been allocated with kzalloc so I'd argue we don't need this to be explicitly
> set at all. Seems like a natural and safe default.
Yes, I forgot it. Thanks for pointing it out.
Ming
>
> >
> > dev = &cxlmd->dev;
> > device_initialize(dev);
>
>
On Mon, Feb 02, 2026 at 02:41:03PM +0000, Jonathan Cameron wrote:
> On Sun, 1 Feb 2026 17:30:01 +0800
> Li Ming <ming.li@zohomail.com> wrote:
>
> > CXL testing environment can trigger following trace
> >
> > Oops: general protection fault, probably for non-canonical address 0xdffffc0000000092: 0000 [#1] SMP KASAN NOPTI
> > KASAN: null-ptr-deref in range [0x0000000000000490-0x0000000000000497]
> > RIP: 0010:cxl_dpa_to_region+0x105/0x1f0 [cxl_core]
> > Call Trace:
> > <TASK>
> > cxl_event_trace_record+0xd1/0xa70 [cxl_core]
> > __cxl_event_trace_record+0x12f/0x1e0 [cxl_core]
> > cxl_mem_get_records_log+0x261/0x500 [cxl_core]
> > cxl_mem_get_event_records+0x7c/0xc0 [cxl_core]
> > cxl_mock_mem_probe+0xd38/0x1c60 [cxl_mock_mem]
> > platform_probe+0x9d/0x130
> > really_probe+0x1c8/0x960
> > __driver_probe_device+0x187/0x3e0
> > driver_probe_device+0x45/0x120
> > __device_attach_driver+0x15d/0x280
> >
> > commit 29317f8dc6ed ("cxl/mem: Introduce cxl_memdev_attach for CXL-dependent operation")
> > initializes cxlmd->endpoint to ERR_PTR(-ENXIO) in cxl_memdev_alloc().
> > However, cxl_dpa_to_region() treats a non-NULL cxlmd->endpoint as a
> > valid endpoint.
> >
> > Across the CXL core, endpoint availability is generally determined by
> > checking whether it is NULL. Align with this convention by initializing
> > cxlmd->endpoint to NULL by default.
>
> I had a look at whether it made sense to use use IS_ERR_OR_NULL() to check
> for validity of the endpoint, but it would be somewhat fiddly and I think
> you are correct that convention here seems to be NULL means not set.
> We don't need the error code. One comment inline.
>
doing validity checks on pointers by checking for null is a pretty
common convention kernel-wide, I would consider setting some structure's
value to an ERR_PTR to be the aberration.
So yeah, good catch
Reviewed-by: Gregory Price <gourry@gourry.net>
~Gregory
© 2016 - 2026 Red Hat, Inc.