[PATCH v7 0/7] cxl: Sashiko bug fixes

Richard Cheng posted 7 patches 3 weeks, 3 days ago
drivers/cxl/core/core.h     |  8 +++---
drivers/cxl/core/edac.c     | 30 +++++++++++++-------
drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
drivers/cxl/core/memdev.c   |  2 ++
drivers/cxl/core/region.c   | 15 +++++-----
5 files changed, 71 insertions(+), 40 deletions(-)
[PATCH v7 0/7] cxl: Sashiko bug fixes
Posted by Richard Cheng 3 weeks, 3 days ago
Seven independent, pre-existing bugs in the CXL core, reported by
sashiko.

Patch 1: Get/Set Feature derive each mailbox command's offset from the
starting offset plus the amount of data already transferred, then store
it in a 16-bit field. A large offset/count supplied through fwctl can
cause a later offset to exceed the representable feature extent and be
truncated by cpu_to_le16(), targeting the wrong feature data. Reject
invalid ranges up front.

Change cxl_get_feature() to return ssize_t so invalid input and mailbox
failures are reported as negative errno instead of being conflated with
a zero-byte result. Update all EDAC callers for the signed return
contract while preserving the existing fwctl RPC response behavior.

Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
the first fully-mapped partition, silently skipping unmapped poison in
all later partitions. Skip that partition instead.

Patch 3: the same function tolerated the -EFAULT a RAM partition returns
for Get Poison List but left it in rc, so a benign fault on the last
scanned partition surfaced as a spurious read failure. Clear rc, as
poison_by_decoder() already does.

Patch 4: the same function also ignored the ctx->offset handoff from
poison_by_decoder() and derived its scan start from the highest DPA
allocation, so the DPA of allocated-but-uncommitted decoders was never
scanned by either phase. Resume the sweep at ctx->offset.

Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
query, so an earlier partition's failure was masked by a later success
and unscanned poison was reported as a clean list. Stop on any error
not tolerated as a RAM -EFAULT.

Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
was assigned to a partition. Malformed device DPA metadata can leave
dpa_res set while part remains -1, causing a poison scan to access
before the partition array. Reject such decoders before the lookup.

Patch 7: the Get and Set Feature fwctl handlers converted all helper
failures into normal RPC responses, sometimes with a SUCCESS device
status. Propagate delivery failures as ioctl errors while continuing to
report actual device errors through rpc_out->retval.

A nonzero short Get Feature response is valid when Offset + Count runs
past the end of the feature. Preserve the returned bytes as a successful
partial transfer, reject unexpected zero-length success responses, and
require fixed-format EDAC callers to receive their complete attribute
structures before consuming them.

Changes since v6 [1]:
- Patch 7: Remove redundant braces. (Dave Jiang)

[1]:
https://lore.kernel.org/linux-cxl/20260826014508.9989-1-icheng@nvidia.com/


Richard Cheng (7):
  cxl/features: Reject feature offset that overflows 16-bit field
  cxl/region: Scan all partitions for unmapped poison
  cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
  cxl/region: Start unmapped poison scan at the committed decoder
    boundary
  cxl/memdev: Don't overwrite the error from an earlier partition poison
    query
  cxl/region: Reject poison scan for decoder without a partition
  cxl/fwctl: Propagate feature RPC delivery errors

 drivers/cxl/core/core.h     |  8 +++---
 drivers/cxl/core/edac.c     | 30 +++++++++++++-------
 drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
 drivers/cxl/core/memdev.c   |  2 ++
 drivers/cxl/core/region.c   | 15 +++++-----
 5 files changed, 71 insertions(+), 40 deletions(-)


base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.53.0
Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
Posted by Alison Schofield 3 weeks, 1 day ago
On Wed, Sep 02, 2026 at 01:38:32PM +0800, Richard Cheng wrote:
> Seven independent, pre-existing bugs in the CXL core, reported by
> sashiko.

Hi Richard,

I finally got around to looking into all this poisonous code ;)
because I had an inkling that they couldn't be independent nor
coincidence based on that common Fixes Tag. That actually is
the issue, aka the finding.

be5cbd084027 is described as an enum removal. Mostly it did that,
CXL_DECODER_PMEM became CXL_PARTMODE_PMEM and nothing else moves. 
But three functions were not renames, they were rewritten.

  cxl_get_poison_by_memdev()	two if blocks became one loop
  cxl_get_poison_unmapped()	rewritten, no line survives
  poison_by_decoder()		gained an unguarded part[] index

If you line up your patches against the pre-rewrite code you'll see
that each one is putting back a statement the rewrite dropped:

  patch 2   'if (!length) return 0' was correct because pmem was the
            last partition.  It became a mid-loop break.
  patch 3   the RAM branch cleared rc on a tolerated -EFAULT.  The
            rewrite continues without clearing it.
  patch 4   the function consumed ctx->offset.  The rewrite derives
            the resume point from the resource child list instead --
            note poison_by_decoder() still computes ctx->offset and
            nothing reads it.  That dead store is the tell.
  patch 5   'if (rc) return rc' lived inside the pmem if block.  When
            the two blocks were merged into a loop it went with the
            block and was never re-added.
  patch 6   be5cbd084027 added six new cxlds->part[cxled->part]
            dereferences and guarded five of them against part < 0.
            In cxl_region_attach() it reordered the existing checks to
            get the guard ahead of the index.  poison_by_decoder() is
            the one site it missed.

BTW - I did confirm it was not a merge issue.

Here's what I'm thinking. DaveJ can pluck patch 1 and 7 separately and
you can take another pass at this as a 'Poison Repair Set'. 

I walked cxl_get_poison_unmapped() against the pre-be5cbd084027 form, but
not the other 2 rewritten functions. It would be good if you can do that.
so we can be sure nothing else is missing. Doing that, along with
addressing Sashiko citings, will get us to the finish line on this one.

I'm going to reply inline to Patches 5 and 6, but here is the
highlights:

- Patches 2, 3 and 4 together restore cxl_get_poison_unmapped()
  and improve upon it. It is partition-generic now and no longer issues
  a zero-length Get Poison List. 

- Patch 5 fixes the masked error but loses the poison records that the
  pre-rewrite code collected before it reported the failure.

- Patch 6 fixes the out-of-bounds read by failing the whole scan for
  the memdev, where the pre-rewrite code scanned that decoder and
  carried on.

-- Alison

> 
> Patch 1: Get/Set Feature derive each mailbox command's offset from the
> starting offset plus the amount of data already transferred, then store
> it in a 16-bit field. A large offset/count supplied through fwctl can
> cause a later offset to exceed the representable feature extent and be
> truncated by cpu_to_le16(), targeting the wrong feature data. Reject
> invalid ranges up front.
> 
> Change cxl_get_feature() to return ssize_t so invalid input and mailbox
> failures are reported as negative errno instead of being conflated with
> a zero-byte result. Update all EDAC callers for the signed return
> contract while preserving the existing fwctl RPC response behavior.
> 
> Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
> the first fully-mapped partition, silently skipping unmapped poison in
> all later partitions. Skip that partition instead.
> 
> Patch 3: the same function tolerated the -EFAULT a RAM partition returns
> for Get Poison List but left it in rc, so a benign fault on the last
> scanned partition surfaced as a spurious read failure. Clear rc, as
> poison_by_decoder() already does.
> 
> Patch 4: the same function also ignored the ctx->offset handoff from
> poison_by_decoder() and derived its scan start from the highest DPA
> allocation, so the DPA of allocated-but-uncommitted decoders was never
> scanned by either phase. Resume the sweep at ctx->offset.
> 
> Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
> query, so an earlier partition's failure was masked by a later success
> and unscanned poison was reported as a clean list. Stop on any error
> not tolerated as a RAM -EFAULT.
> 
> Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
> was assigned to a partition. Malformed device DPA metadata can leave
> dpa_res set while part remains -1, causing a poison scan to access
> before the partition array. Reject such decoders before the lookup.
> 
> Patch 7: the Get and Set Feature fwctl handlers converted all helper
> failures into normal RPC responses, sometimes with a SUCCESS device
> status. Propagate delivery failures as ioctl errors while continuing to
> report actual device errors through rpc_out->retval.
> 
> A nonzero short Get Feature response is valid when Offset + Count runs
> past the end of the feature. Preserve the returned bytes as a successful
> partial transfer, reject unexpected zero-length success responses, and
> require fixed-format EDAC callers to receive their complete attribute
> structures before consuming them.
> 
> Changes since v6 [1]:
> - Patch 7: Remove redundant braces. (Dave Jiang)
> 
> [1]:
> https://lore.kernel.org/linux-cxl/20260826014508.9989-1-icheng@nvidia.com/
> 
> 
> Richard Cheng (7):
>   cxl/features: Reject feature offset that overflows 16-bit field
>   cxl/region: Scan all partitions for unmapped poison
>   cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
>   cxl/region: Start unmapped poison scan at the committed decoder
>     boundary
>   cxl/memdev: Don't overwrite the error from an earlier partition poison
>     query
>   cxl/region: Reject poison scan for decoder without a partition
>   cxl/fwctl: Propagate feature RPC delivery errors
> 
>  drivers/cxl/core/core.h     |  8 +++---
>  drivers/cxl/core/edac.c     | 30 +++++++++++++-------
>  drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
>  drivers/cxl/core/memdev.c   |  2 ++
>  drivers/cxl/core/region.c   | 15 +++++-----
>  5 files changed, 71 insertions(+), 40 deletions(-)
> 
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> -- 
> 2.53.0
>
Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
Posted by Richard Cheng 1 week, 4 days ago
On Thu, Sep 03, 2026 at 10:20:07PM +0800, Alison Schofield wrote:
> On Wed, Sep 02, 2026 at 01:38:32PM +0800, Richard Cheng wrote:
> > Seven independent, pre-existing bugs in the CXL core, reported by
> > sashiko.
> 
> Hi Richard,
> 
> I finally got around to looking into all this poisonous code ;)
> because I had an inkling that they couldn't be independent nor
> coincidence based on that common Fixes Tag. That actually is
> the issue, aka the finding.
> 
> be5cbd084027 is described as an enum removal. Mostly it did that,
> CXL_DECODER_PMEM became CXL_PARTMODE_PMEM and nothing else moves. 
> But three functions were not renames, they were rewritten.
> 
>   cxl_get_poison_by_memdev()	two if blocks became one loop
>   cxl_get_poison_unmapped()	rewritten, no line survives
>   poison_by_decoder()		gained an unguarded part[] index
> 
> If you line up your patches against the pre-rewrite code you'll see
> that each one is putting back a statement the rewrite dropped:
> 
>   patch 2   'if (!length) return 0' was correct because pmem was the
>             last partition.  It became a mid-loop break.
>   patch 3   the RAM branch cleared rc on a tolerated -EFAULT.  The
>             rewrite continues without clearing it.
>   patch 4   the function consumed ctx->offset.  The rewrite derives
>             the resume point from the resource child list instead --
>             note poison_by_decoder() still computes ctx->offset and
>             nothing reads it.  That dead store is the tell.
>   patch 5   'if (rc) return rc' lived inside the pmem if block.  When
>             the two blocks were merged into a loop it went with the
>             block and was never re-added.
>   patch 6   be5cbd084027 added six new cxlds->part[cxled->part]
>             dereferences and guarded five of them against part < 0.
>             In cxl_region_attach() it reordered the existing checks to
>             get the guard ahead of the index.  poison_by_decoder() is
>             the one site it missed.
> 
> BTW - I did confirm it was not a merge issue.
> 
> Here's what I'm thinking. DaveJ can pluck patch 1 and 7 separately and
> you can take another pass at this as a 'Poison Repair Set'. 
> 
> I walked cxl_get_poison_unmapped() against the pre-be5cbd084027 form, but
> not the other 2 rewritten functions. It would be good if you can do that.
> so we can be sure nothing else is missing. Doing that, along with
> addressing Sashiko citings, will get us to the finish line on this one.
> 
> I'm going to reply inline to Patches 5 and 6, but here is the
> highlights:
> 
> - Patches 2, 3 and 4 together restore cxl_get_poison_unmapped()
>   and improve upon it. It is partition-generic now and no longer issues
>   a zero-length Get Poison List. 
> 
> - Patch 5 fixes the masked error but loses the poison records that the
>   pre-rewrite code collected before it reported the failure.
> 
> - Patch 6 fixes the out-of-bounds read by failing the whole scan for
>   the memdev, where the pre-rewrite code scanned that decoder and
>   carried on.
> 
> -- Alison
>

Hi Alison,

Thanks a lot for your detailed reviewed.

I have a few questions to discuss with you before I sent another patch series
for poison rework.

So for patch 5's issue, if we get RAM query error and PMEM query error at the same
time, which return value would you prefer ? I think retained all the record in
some data structure wouldn't be an issue, just the return error, you would like to see
the first error or the last ?

and for patch 6, I see it and will addressed it in the new series.

Best regards,
Richard Cheng.
 
> > 
> > Patch 1: Get/Set Feature derive each mailbox command's offset from the
> > starting offset plus the amount of data already transferred, then store
> > it in a 16-bit field. A large offset/count supplied through fwctl can
> > cause a later offset to exceed the representable feature extent and be
> > truncated by cpu_to_le16(), targeting the wrong feature data. Reject
> > invalid ranges up front.
> > 
> > Change cxl_get_feature() to return ssize_t so invalid input and mailbox
> > failures are reported as negative errno instead of being conflated with
> > a zero-byte result. Update all EDAC callers for the signed return
> > contract while preserving the existing fwctl RPC response behavior.
> > 
> > Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
> > the first fully-mapped partition, silently skipping unmapped poison in
> > all later partitions. Skip that partition instead.
> > 
> > Patch 3: the same function tolerated the -EFAULT a RAM partition returns
> > for Get Poison List but left it in rc, so a benign fault on the last
> > scanned partition surfaced as a spurious read failure. Clear rc, as
> > poison_by_decoder() already does.
> > 
> > Patch 4: the same function also ignored the ctx->offset handoff from
> > poison_by_decoder() and derived its scan start from the highest DPA
> > allocation, so the DPA of allocated-but-uncommitted decoders was never
> > scanned by either phase. Resume the sweep at ctx->offset.
> > 
> > Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
> > query, so an earlier partition's failure was masked by a later success
> > and unscanned poison was reported as a clean list. Stop on any error
> > not tolerated as a RAM -EFAULT.
> > 
> > Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
> > was assigned to a partition. Malformed device DPA metadata can leave
> > dpa_res set while part remains -1, causing a poison scan to access
> > before the partition array. Reject such decoders before the lookup.
> > 
> > Patch 7: the Get and Set Feature fwctl handlers converted all helper
> > failures into normal RPC responses, sometimes with a SUCCESS device
> > status. Propagate delivery failures as ioctl errors while continuing to
> > report actual device errors through rpc_out->retval.
> > 
> > A nonzero short Get Feature response is valid when Offset + Count runs
> > past the end of the feature. Preserve the returned bytes as a successful
> > partial transfer, reject unexpected zero-length success responses, and
> > require fixed-format EDAC callers to receive their complete attribute
> > structures before consuming them.
> > 
> > Changes since v6 [1]:
> > - Patch 7: Remove redundant braces. (Dave Jiang)
> > 
> > [1]:
> > https://lore.kernel.org/linux-cxl/20260826014508.9989-1-icheng@nvidia.com/
> > 
> > 
> > Richard Cheng (7):
> >   cxl/features: Reject feature offset that overflows 16-bit field
> >   cxl/region: Scan all partitions for unmapped poison
> >   cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
> >   cxl/region: Start unmapped poison scan at the committed decoder
> >     boundary
> >   cxl/memdev: Don't overwrite the error from an earlier partition poison
> >     query
> >   cxl/region: Reject poison scan for decoder without a partition
> >   cxl/fwctl: Propagate feature RPC delivery errors
> > 
> >  drivers/cxl/core/core.h     |  8 +++---
> >  drivers/cxl/core/edac.c     | 30 +++++++++++++-------
> >  drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
> >  drivers/cxl/core/memdev.c   |  2 ++
> >  drivers/cxl/core/region.c   | 15 +++++-----
> >  5 files changed, 71 insertions(+), 40 deletions(-)
> > 
> > 
> > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > -- 
> > 2.53.0
> >
Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
Posted by Alison Schofield 1 week, 3 days ago
On Tue, Sep 15, 2026 at 04:11:27PM +0800, Richard Cheng wrote:
> On Thu, Sep 03, 2026 at 10:20:07PM +0800, Alison Schofield wrote:
> > On Wed, Sep 02, 2026 at 01:38:32PM +0800, Richard Cheng wrote:
> > > Seven independent, pre-existing bugs in the CXL core, reported by
> > > sashiko.
> > 
> > Hi Richard,
> > 
> > I finally got around to looking into all this poisonous code ;)
> > because I had an inkling that they couldn't be independent nor
> > coincidence based on that common Fixes Tag. That actually is
> > the issue, aka the finding.
> > 
> > be5cbd084027 is described as an enum removal. Mostly it did that,
> > CXL_DECODER_PMEM became CXL_PARTMODE_PMEM and nothing else moves. 
> > But three functions were not renames, they were rewritten.
> > 
> >   cxl_get_poison_by_memdev()	two if blocks became one loop
> >   cxl_get_poison_unmapped()	rewritten, no line survives
> >   poison_by_decoder()		gained an unguarded part[] index
> > 
> > If you line up your patches against the pre-rewrite code you'll see
> > that each one is putting back a statement the rewrite dropped:
> > 
> >   patch 2   'if (!length) return 0' was correct because pmem was the
> >             last partition.  It became a mid-loop break.
> >   patch 3   the RAM branch cleared rc on a tolerated -EFAULT.  The
> >             rewrite continues without clearing it.
> >   patch 4   the function consumed ctx->offset.  The rewrite derives
> >             the resume point from the resource child list instead --
> >             note poison_by_decoder() still computes ctx->offset and
> >             nothing reads it.  That dead store is the tell.
> >   patch 5   'if (rc) return rc' lived inside the pmem if block.  When
> >             the two blocks were merged into a loop it went with the
> >             block and was never re-added.
> >   patch 6   be5cbd084027 added six new cxlds->part[cxled->part]
> >             dereferences and guarded five of them against part < 0.
> >             In cxl_region_attach() it reordered the existing checks to
> >             get the guard ahead of the index.  poison_by_decoder() is
> >             the one site it missed.
> > 
> > BTW - I did confirm it was not a merge issue.
> > 
> > Here's what I'm thinking. DaveJ can pluck patch 1 and 7 separately and
> > you can take another pass at this as a 'Poison Repair Set'. 
> > 
> > I walked cxl_get_poison_unmapped() against the pre-be5cbd084027 form, but
> > not the other 2 rewritten functions. It would be good if you can do that.
> > so we can be sure nothing else is missing. Doing that, along with
> > addressing Sashiko citings, will get us to the finish line on this one.
> > 
> > I'm going to reply inline to Patches 5 and 6, but here is the
> > highlights:
> > 
> > - Patches 2, 3 and 4 together restore cxl_get_poison_unmapped()
> >   and improve upon it. It is partition-generic now and no longer issues
> >   a zero-length Get Poison List. 
> > 
> > - Patch 5 fixes the masked error but loses the poison records that the
> >   pre-rewrite code collected before it reported the failure.
> > 
> > - Patch 6 fixes the out-of-bounds read by failing the whole scan for
> >   the memdev, where the pre-rewrite code scanned that decoder and
> >   carried on.
> > 
> > -- Alison
> >
> 
> Hi Alison,
> 
> Thanks a lot for your detailed reviewed.
> 
> I have a few questions to discuss with you before I sent another patch series
> for poison rework.
> 
> So for patch 5's issue, if we get RAM query error and PMEM query error at the same
> time, which return value would you prefer ? I think retained all the record in
> some data structure wouldn't be an issue, just the return error, you would like to see
> the first error or the last ?

Hi Richard,

Neither! Changed my mind after a closer look and seeing that all the query sites
are stopping on first error, so your patch, as is, is inline with other queries.

However, I do think we could do better than that, but that would be beyond the
scope of this fixup patch set. We could change our policy to always query both
partitions and return the first error rather than stopping on it.

So I think you are heading to leaving Patch 5 as is, fixing up Patch 6 and taking
a done on this Sashiko poison 'fixups' set.

-- Alison




> 
> and for patch 6, I see it and will addressed it in the new series.
> 
> Best regards,
> Richard Cheng.
>  
> > > 
> > > Patch 1: Get/Set Feature derive each mailbox command's offset from the
> > > starting offset plus the amount of data already transferred, then store
> > > it in a 16-bit field. A large offset/count supplied through fwctl can
> > > cause a later offset to exceed the representable feature extent and be
> > > truncated by cpu_to_le16(), targeting the wrong feature data. Reject
> > > invalid ranges up front.
> > > 
> > > Change cxl_get_feature() to return ssize_t so invalid input and mailbox
> > > failures are reported as negative errno instead of being conflated with
> > > a zero-byte result. Update all EDAC callers for the signed return
> > > contract while preserving the existing fwctl RPC response behavior.
> > > 
> > > Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
> > > the first fully-mapped partition, silently skipping unmapped poison in
> > > all later partitions. Skip that partition instead.
> > > 
> > > Patch 3: the same function tolerated the -EFAULT a RAM partition returns
> > > for Get Poison List but left it in rc, so a benign fault on the last
> > > scanned partition surfaced as a spurious read failure. Clear rc, as
> > > poison_by_decoder() already does.
> > > 
> > > Patch 4: the same function also ignored the ctx->offset handoff from
> > > poison_by_decoder() and derived its scan start from the highest DPA
> > > allocation, so the DPA of allocated-but-uncommitted decoders was never
> > > scanned by either phase. Resume the sweep at ctx->offset.
> > > 
> > > Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
> > > query, so an earlier partition's failure was masked by a later success
> > > and unscanned poison was reported as a clean list. Stop on any error
> > > not tolerated as a RAM -EFAULT.
> > > 
> > > Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
> > > was assigned to a partition. Malformed device DPA metadata can leave
> > > dpa_res set while part remains -1, causing a poison scan to access
> > > before the partition array. Reject such decoders before the lookup.
> > > 
> > > Patch 7: the Get and Set Feature fwctl handlers converted all helper
> > > failures into normal RPC responses, sometimes with a SUCCESS device
> > > status. Propagate delivery failures as ioctl errors while continuing to
> > > report actual device errors through rpc_out->retval.
> > > 
> > > A nonzero short Get Feature response is valid when Offset + Count runs
> > > past the end of the feature. Preserve the returned bytes as a successful
> > > partial transfer, reject unexpected zero-length success responses, and
> > > require fixed-format EDAC callers to receive their complete attribute
> > > structures before consuming them.
> > > 
> > > Changes since v6 [1]:
> > > - Patch 7: Remove redundant braces. (Dave Jiang)
> > > 
> > > [1]:
> > > https://lore.kernel.org/linux-cxl/20260826014508.9989-1-icheng@nvidia.com/
> > > 
> > > 
> > > Richard Cheng (7):
> > >   cxl/features: Reject feature offset that overflows 16-bit field
> > >   cxl/region: Scan all partitions for unmapped poison
> > >   cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
> > >   cxl/region: Start unmapped poison scan at the committed decoder
> > >     boundary
> > >   cxl/memdev: Don't overwrite the error from an earlier partition poison
> > >     query
> > >   cxl/region: Reject poison scan for decoder without a partition
> > >   cxl/fwctl: Propagate feature RPC delivery errors
> > > 
> > >  drivers/cxl/core/core.h     |  8 +++---
> > >  drivers/cxl/core/edac.c     | 30 +++++++++++++-------
> > >  drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
> > >  drivers/cxl/core/memdev.c   |  2 ++
> > >  drivers/cxl/core/region.c   | 15 +++++-----
> > >  5 files changed, 71 insertions(+), 40 deletions(-)
> > > 
> > > 
> > > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > > -- 
> > > 2.53.0
> > >
Re: [PATCH v7 0/7] cxl: Sashiko bug fixes
Posted by Dave Jiang 3 weeks ago

On 9/1/26 10:38 PM, Richard Cheng wrote:
> Seven independent, pre-existing bugs in the CXL core, reported by
> sashiko.
> 
> Patch 1: Get/Set Feature derive each mailbox command's offset from the
> starting offset plus the amount of data already transferred, then store
> it in a 16-bit field. A large offset/count supplied through fwctl can
> cause a later offset to exceed the representable feature extent and be
> truncated by cpu_to_le16(), targeting the wrong feature data. Reject
> invalid ranges up front.
> 
> Change cxl_get_feature() to return ssize_t so invalid input and mailbox
> failures are reported as negative errno instead of being conflated with
> a zero-byte result. Update all EDAC callers for the signed return
> contract while preserving the existing fwctl RPC response behavior.
> 
> Patch 2: cxl_get_poison_unmapped() aborted its whole partition sweep on
> the first fully-mapped partition, silently skipping unmapped poison in
> all later partitions. Skip that partition instead.
> 
> Patch 3: the same function tolerated the -EFAULT a RAM partition returns
> for Get Poison List but left it in rc, so a benign fault on the last
> scanned partition surfaced as a spurious read failure. Clear rc, as
> poison_by_decoder() already does.
> 
> Patch 4: the same function also ignored the ctx->offset handoff from
> poison_by_decoder() and derived its scan start from the highest DPA
> allocation, so the DPA of allocated-but-uncommitted decoders was never
> scanned by either phase. Resume the sweep at ctx->offset.
> 
> Patch 5: cxl_get_poison_by_memdev() overwrote rc on each partition
> query, so an earlier partition's failure was masked by a later success
> and unscanned poison was reported as a clean list. Stop on any error
> not tolerated as a RAM -EFAULT.
> 
> Patch 6: poison_by_decoder() assumed every decoder with a DPA reservation
> was assigned to a partition. Malformed device DPA metadata can leave
> dpa_res set while part remains -1, causing a poison scan to access
> before the partition array. Reject such decoders before the lookup.
> 
> Patch 7: the Get and Set Feature fwctl handlers converted all helper
> failures into normal RPC responses, sometimes with a SUCCESS device
> status. Propagate delivery failures as ioctl errors while continuing to
> report actual device errors through rpc_out->retval.
> 
> A nonzero short Get Feature response is valid when Offset + Count runs
> past the end of the feature. Preserve the returned bytes as a successful
> partial transfer, reject unexpected zero-length success responses, and
> require fixed-format EDAC callers to receive their complete attribute
> structures before consuming them.
> 
> Changes since v6 [1]:
> - Patch 7: Remove redundant braces. (Dave Jiang)
> 
> [1]:
> https://lore.kernel.org/linux-cxl/20260826014508.9989-1-icheng@nvidia.com/
> 
> 
> Richard Cheng (7):
>   cxl/features: Reject feature offset that overflows 16-bit field
>   cxl/region: Scan all partitions for unmapped poison
>   cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
>   cxl/region: Start unmapped poison scan at the committed decoder
>     boundary
>   cxl/memdev: Don't overwrite the error from an earlier partition poison
>     query
>   cxl/region: Reject poison scan for decoder without a partition
>   cxl/fwctl: Propagate feature RPC delivery errors
> 
>  drivers/cxl/core/core.h     |  8 +++---
>  drivers/cxl/core/edac.c     | 30 +++++++++++++-------
>  drivers/cxl/core/features.c | 56 ++++++++++++++++++++++++-------------
>  drivers/cxl/core/memdev.c   |  2 ++
>  drivers/cxl/core/region.c   | 15 +++++-----
>  5 files changed, 71 insertions(+), 40 deletions(-)
> 
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07

Patches 1 and 7 applied to cxl/next
51493ff66ad6
3eb3376944ac