Re: [PATCH v12 6/8] cxl/mem: Configure dynamic capacity interrupts

On Fri, Jul 31, 2026 at 09:04:43AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Infinite IRQ Thread Loop / CPU Spin when handling DCD events
> 
False Positive. DCD events are ignored because DCD is turned off
(mds->dcd_supported = false).

> Pre-existing issues:
> - [High] Out-of-Bounds Read and Information Leak in Event logs
> - [High] Out-of-Bounds Read and Information Leak in Poison logs
> --
> 
> --- Patch [6]: [PATCH v12 6/8] cxl/mem: Configure dynamic capacity interrupts ---
> commit 5b173c122c3602992c4ceeafd1c2d87a80accacb
> Author: Ira Weiny <[email protected]>
> 
> cxl/mem: Configure dynamic capacity interrupts
> 
> Dynamic Capacity Devices (DCD) support extent change notifications
> through the event log mechanism. Configure DCD event log interrupts
> on devices supporting dynamic capacity. Disable DCD if interrupts
> are not supported.
> 
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index df13fb8802c38..6cb344ec4f3a7 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -509,11 +509,30 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
> >  	return true;
> >  }
> >  
> > +/*
> > + * Event logs the driver drains: standard logs when native_cxl, DCD when
> > + * supported.
> > + */
> > +static u32 cxl_event_drain_mask(struct pci_host_bridge *host_bridge,
> > +				struct cxl_memdev_state *mds)
> > +{
> > +	u32 mask = 0;
> > +
> > +	if (host_bridge->native_cxl_error)
> > +		mask |= CXLDEV_EVENT_STATUS_ALL & ~CXLDEV_EVENT_STATUS_DCD;
> > +	if (cxl_dcd_supported(mds))
> > +		mask |= CXLDEV_EVENT_STATUS_DCD;
> > +	return mask;
> > +}
> > +
> >  static irqreturn_t cxl_event_thread(int irq, void *id)
> >  {
> >  	struct cxl_dev_id *dev_id = id;
> >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > +	struct pci_host_bridge *host_bridge =
> > +		pci_find_host_bridge(to_pci_dev(cxlds->dev)->bus);
> > +	u32 mask = cxl_event_drain_mask(host_bridge, mds);
> >  	u32 status;
> >  
> >  	do {
> > @@ -522,8 +541,8 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> >  		 * ignore the reserved upper 32 bits
> >  		 */
> >  		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
> > -		/* Ignore logs unknown to the driver */
> > -		status &= CXLDEV_EVENT_STATUS_ALL;
> > +		/* Ignore logs unknown to the driver or owned by BIOS */
> > +		status &= mask;
> >  		if (!status)
> >  			break;
> >  		cxl_mem_get_event_records(mds, status);
> 
> [Severity: High]
> Since mask now includes CXLDEV_EVENT_STATUS_DCD, status can have this bit set.
> However, cxl_mem_get_event_records() does not appear to handle the DCD event:
> 
The mask never includes CXLDEV_EVENT_STATUS_DCD.
Above in cxl_event_drain_mask:

if (cxl_dcd_supported(mds))
              mask |= CXLDEV_EVENT_STATUS_DCD;

mds->dcd_supported is set to false in Patch 1 so the DCD bit is never
set.

So status &= mask becomes zero and we break from the loop.

- Anisa
> drivers/cxl/core/mbox.c:cxl_mem_get_event_records() {
>     ...
>     if (status & CXLDEV_EVENT_STATUS_FATAL)
>         cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FATAL);
>     if (status & CXLDEV_EVENT_STATUS_FAIL)
>         cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FAIL);
>     if (status & CXLDEV_EVENT_STATUS_WARN)
>         cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_WARN);
>     if (status & CXLDEV_EVENT_STATUS_INFO)
>         cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_INFO);
>     ...
> }
> 
> Will this fail to clear the DCD event from the device? If the event is never
> cleared, will the hardware register retain the bit and cause the while (status)
> loop to spin infinitely on the exact same status bit, potentially causing a CPU
> soft lockup?
> 
> >  		cond_resched();
> >  	} while (status);
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential out-of-bounds read when
> fetching event logs:
> 
> drivers/cxl/core/mbox.c:cxl_mem_get_records_log() {
>     ...
>     nr_rec = le16_to_cpu(payload->record_count);
>     if (!nr_rec)
>         break;
> 
>     for (i = 0; i < nr_rec; i++)
>         __cxl_event_trace_record(cxlmd, type,
>                                  &payload->records[i]);
>     ...
> }
> 
> Does this code validate the hardware-provided nr_rec against the allocated
> payload size? If an untrusted or compromised device returns an artificially
> large count, could this read out of bounds?
> 
> Similarly, when clearing those records:
> 
> drivers/cxl/core/mbox.c:cxl_clear_event_record() {
>     ...
>     for (cnt = 0; cnt < total; cnt++) {
>         struct cxl_event_record_raw *raw = &get_pl->records[cnt];
>         struct cxl_event_generic *gen = &raw->event.generic;
> 
>         payload->handles[i++] = gen->hdr.handle;
>     ...
> }
> 
> Could this write the out-of-bounds memory values into the clear event payload
> sent back to the device, leading to an information leak?
> 
> [Severity: High]
> This is also a pre-existing issue, but a similar pattern exists in the poison
> log handling:
> 
> drivers/cxl/core/mbox.c:cxl_mem_get_poison() {
>     ...
>     for (int i = 0; i < le16_to_cpu(po->count); i++)
>         trace_cxl_poison(cxlmd, cxlr, &po->record[i],
>                          po->flags, po->overflow_ts,
>                          CXL_POISON_TRACE_LIST);
>     ...
> }
> 
> Does this validate po->count against the buffer size, or could it result
> in an out-of-bounds read if the device returns a large count?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6