Re: [PATCH 3/3] cxl/hdm: Restore commit_end when decoder enumeration fails

On Mon, Jul 13, 2026 at 08:58:20PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Transient reset of `port->commit_end` allows concurrent bypass of safety checks in `cxl_mem_sanitize`, potentially destroying active memory.

Fixed in v2.
Moved reset from before the loop to the failure patch only.
Success path does not touch commit_end so no transient state exists.

> --
> 
> --- Patch [3]: [PATCH 3/3] cxl/hdm: Restore commit_end when decoder enumeration fails ---
> commit 0d7f87c9ca0ba7b9bbdd31e5ea90994445b1c9b5
> Author: Alison Schofield <[email protected]>
> 
> cxl/hdm: Restore commit_end when decoder enumeration fails
> 
> This commit resets port->commit_end to -1 before enumerating decoders so
> the commit state is rebuilt from the decoders actually committed on each
> probe. This prevents previous enumeration failures from causing subsequent
> probes to fail.
> 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 75d17120233da..0145d3db31070 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -1178,6 +1178,10 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
> >  
> >  	cxl_settle_decoders(cxlhdm);
> >  
> > +	/* Reset to the none committed baseline for a fresh enumeration */
> > +	scoped_guard(rwsem_write, &cxl_rwsem.region)
> > +		port->commit_end = -1;
> 
> [Severity: High]
> Does this scoped_guard drop the lock too early?
> 
> Because scoped_guard only holds the lock for the following single statement,
> the rwsem_write lock is released immediately after setting port->commit_end
> to -1. This leaves the transient -1 state exposed to readers without protection
> while the loop below performs the hardware enumeration.
> 
> Could a concurrent reader like cxl_mem_sanitize() acquire the read lock
> during this window, observe a temporarily empty committed decoder count
> (since cxl_num_decoders_committed() evaluates the exposed -1), and 
> erroneously bypass its safety checks to securely erase actively mapped memory?
> 
> > +
> >  	for (i = 0; i < cxlhdm->decoder_count; i++) {
> >  		int rc, target_count = cxlhdm->target_count;
> >  		struct cxl_decoder *cxld;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3