linux-kernel - Re: [RFC PATCH] cxl: Fix premature commit_end increment on decoder commit failure

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aXmz_pH92dL1TqJ-@U-Q2G32XWJ-0138.local>
Date: Wed, 28 Jan 2026 15:00:14 +0800
From: Yuxiong Wang <yuxiong.wang@...ux.alibaba.com>
To: Alison Schofield <alison.schofield@...el.com>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
	vishal.l.verma@...el.com, ira.weiny@...el.com,
	dan.j.williams@...el.com, ming.li@...omail.com, rrichter@....com,
	ying.huang@...ux.alibaba.com, linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] cxl: Fix premature commit_end increment on decoder
 commit failure

Resending with corrected CC list - apologies to those who received the
message twice.

Thanks for comments!

On Tue, Jan 27, 2026 at 01:15:04PM -0800, Alison Schofield wrote:
> On Tue, Jan 27, 2026 at 04:48:55PM +0800, Yuxiong Wang wrote:
> > In cxl_decoder_commit(), commit_end is incremented before verifying whether the
> > commit succeeded, and the CXL_DECODER_F_ENABLE bit in cxld->flags is only set
> > after a successful commit. As a result, if the commit fails, commit_end has been
> > incremented and cxld->reset() has no effect since the flag is not set, so commit_end
> > remains incorrectly incremented. The inconsistency between commit_end and
> > CXL_DECODER_F_ENABLE causes failure during subsequent either commit or reset
> > operations.
> > 
> > Fix this by incrementing commit_end only after confirming the commit succeeded.
> > Since cxld_await_commit() clears the decoder commit bit on failure, no additional
> > reset is required. Remove the ineffective cxld->reset() call in this case.
> 
> 
> Why are the writel()'s that cxld->reset() intended to do not really
> needed? Is only the twiddle of the COMMIT bit needed for a reset or
> do we need to 0 the size and base offsets we previously wrote?

In this case, cxld->reset() does not work because CXL_DECODER_F_ENABLE bit
in cxld->flag is not set. 

The size and base registers shall have no effects if the commit bit is cleared.
And except the reset and commit functions, there are no other functions that
touch these registers after initialization. So I think the key step is to clear
the commit bit, which has been done in cxld_await_commit(), and the size and
base will be overwritten in the next commit.

Considering the potential hardware problems, perhaps it's more a secure
solution to clear all these offsets. I can add a function like
'clear_hw_decoder()' to do this.

> 
> How did you find this?  Maybe we can add test case to cxl unit tests.

I think this is a very rare situation, since the software has done very
strict checks before commits. We found this when testing the cxl module
of kernel and firmware. We committed the region in OS, and faked a host
bridge hdm decoder commit failure in firmware. During the second commit,
we triggered this problem with 'out of order commit'.

> 
> Thanks,
> Alison

Best Regards,
Yuxiong

> 
> 
> > 
> > Fixes: 176baef ("cxl/hdm: Commit decoder state to hardware")
> > Signed-off-by: Yuxiong Wang <yuxiong.wang@...ux.alibaba.com>
> > Acked-by: Huang Ying <ying.huang@...ux.alibaba.com>
> > ---
> >  drivers/cxl/core/hdm.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index eb5a3a7640c6..912f648a6b7a 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -844,14 +844,13 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
> >  	scoped_guard(rwsem_read, &cxl_rwsem.dpa)
> >  		setup_hw_decoder(cxld, hdm);
> >  
> > -	port->commit_end++;
> >  	rc = cxld_await_commit(hdm, cxld->id);
> >  	if (rc) {
> >  		dev_dbg(&port->dev, "%s: error %d committing decoder\n",
> >  			dev_name(&cxld->dev), rc);
> > -		cxld->reset(cxld);
> >  		return rc;
> >  	}
> > +	port->commit_end++;
> >  	cxld->flags |= CXL_DECODER_F_ENABLE;
> >  
> >  	return 0;
> > -- 
> > 2.50.1 (Apple Git-155)
> >