Re: [PATCH v3 1/2] cxl/edac: Read cached DRAM event records under the xarray lock

[Jongwon Lee <ai@linux.com>]

Hi Richard,

Thanks for going through the whole series so carefully. v4 is out
with all three of your comments addressed:
https://lore.kernel.org/r/20261102123233.74598-1-ai@linux.com

On Mon, Nov 02, 2026 at 03:56:17PM +0800, Richard Cheng wrote:
> Did the bug actually happened ? if so , can you provide the error log ?

Only with a test-only delay. Without it, v4 ran for 1800 s and the
unpatched kernel for 900 s, with four repair loops and the host
injecting events with no gap, and neither reported anything. With
mdelay(20) added after xa_load() in cxl_find_rec_dram() and after the
lookup in the sparing path, the unpatched kernel hit this after 120 s
(trimmed):

  BUG: KASAN: slab-use-after-free in cxl_do_mem_sparing+0xed9/0x1000
  Read of size 2 at addr ff1100000670732b by task c1-repair.sh/153
  Call Trace:
   cxl_do_mem_sparing+0xed9/0x1000
   repair_store+0x1d9/0x340
   kernfs_fop_write_iter+0x337/0x510
   vfs_write+0x4a2/0xfc0
   ksys_write+0x111/0x250
   do_syscall_64+0x10e/0x5e0
  Allocated by task 68:
   kmemdup_noprof+0x20/0x40
   cxl_store_rec_dram+0xb0/0x360
   cxl_event_trace_record+0x617/0xd40
   __cxl_event_trace_record+0x14c/0x1d0
   cxl_mem_get_records_log+0x27d/0x540
   cxl_event_thread+0x14e/0x1f0
  Freed by task 68:
   kfree+0x1e3/0x5e0
   cxl_store_rec_dram+0x154/0x360
   cxl_event_trace_record+0x617/0xd40
   __cxl_event_trace_record+0x14c/0x1d0
   cxl_mem_get_records_log+0x27d/0x540
   cxl_event_thread+0x14e/0x1f0
  The buggy address is located 43 bytes inside of
   freed 128-byte region [ff11000006707300, ff11000006707380)

Offset 43 is media_hdr.validity_flags, the second read in the sparing
path, and the record was freed by cxl_store_rec_dram() in the event
thread. I can send the full log if it helps.

So the window is small in practice. The race was reported by Sashiko
from reading the code, and the delayed run confirms the free and the
read can overlap.

> Seems like the root cause is that the cxl's rwsem was only protecting CXL
> topology and DPA, while leaving memory problem's entry ( those xarray entry )
> without protection.

Yes, that matches what I found.

> Using xarray lock is fair for me, keeping the access short and the entry
> already use xarray as bookkeeping, though I'm wonder if there're better
> alternative for locking mechanism. we might want to discuss abit about that.

The other options I looked at were kfree_rcu(), which needs a wrapper
struct with an rcu_head around each cached record, and device_lock() in
the repair path, which can deadlock against unbind. A refcount per
record would also work, but it adds get/put to the event path for a
lookup that copies one small struct. I'm happy to switch if you or the
maintainers prefer one of these.

> What if we copy the whole struct cxl_event_dram ? PPR can continue
> passing NULL since it only needs matching result.

Agreed, that keeps the signature stable if callers need more fields
later. In v4 cxl_find_rec_dram() and cxl_mem_get_rec_dram() take a
struct cxl_event_dram *out, the sparing path reads validity_flags from
its own copy, and PPR passes NULL. The cover letter now describes
patch 1 first, as you asked.

Thanks,
Jongwon