Re: [RFC PATCH 1/5] cxl/mbox: Add Get Poison List snapshot support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl: cxl_poison_snapshot_append returns -ENOSPC, bypassing truncation handler
--

commit 14df359a915714a1d8f08f27fcdf1ccf13b88763
Author: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>

cxl/mbox: Add Get Poison List snapshot support

The commit adds snapshot support to copy Get Poison List records from a
device into a caller-owned buffer. It allows callers to inspect the complete
set of collected records while preserving existing trace-only behavior for
current callers.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 55828a836c01a..2752b46ffee29 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
[ ... ]
> +static int cxl_poison_snapshot_append(struct cxl_poison_snapshot *snapshot,
> +				      struct cxl_poison_record *records,
> +				      u32 count)
> +{
[ ... ]
> +	available = snapshot->capacity - snapshot->nr_records;
> +	if (count > available) {
> +		snapshot->truncated = true;
> +		return -ENOSPC;
> +	}

[Severity: High]
Does returning -ENOSPC here cause the caller to prematurely abort?

In drivers/cxl/core/mbox.c, cxl_poison_snapshot_append(), when the device
returns more poison records than snapshot->capacity, returning -ENOSPC instead
of clamping the count and returning success prevents the loop in
cxl_mem_get_poison_common() from completing its normal termination logic.

> +
> +	memcpy(&snapshot->records[snapshot->nr_records], records,
> +	       count * sizeof(*records));
> +	snapshot->nr_records += count;
> +
> +	return 0;
> +}
> +
> +static int cxl_mem_get_poison_common(struct cxl_memdev *cxlmd, u64 offset,
> +				     u64 len, struct cxl_region *cxlr,
> +				     struct cxl_poison_snapshot *snapshot)
>  {
[ ... ]
> +		rc = cxl_poison_snapshot_append(snapshot, po->record, count);
> +		if (rc)
> +			break;

[Severity: High]
Does breaking early on -ENOSPC deprive the caller of the expected truncation
state?

In drivers/cxl/core/mbox.c, cxl_mem_get_poison_common(), if
cxl_poison_snapshot_append() returns -ENOSPC, this propagates up and breaks the
loop before the internal termination logic can be reached. The generic error
cascades up to the caller, bypassing the expected truncation handler designed
to handle this exact scenario.

> +
> +		/* Protect against an uncleared _FLAG_MORE */
> -		nr_records = nr_records + le16_to_cpu(po->count);
> +		nr_records += count;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010162017.62506-1-shaikhkamal2012@gmail.com?part=1