Re: [PATCH] cxl/edac: Bounds-check ECS threshold index from device

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Sep 29, 2026 at 11:17:42AM +0800, Yili Zhang wrote:
> On 2026-09-28 11:21, Dave Jiang wrote:
> >
> > On 9/28/26 12:35 AM, Yili Zhang wrote:
> > > cxl_get_ecs_threshold() extracts a 3-bit index (0-7) from the
> > > device-supplied ECS config word and uses it to
> > > index ecs_supp_threshold[], which only has 6 elements. A device reporting
> > > index 6 or 7 causes an out-of-bounds read of adjacent .rodata, whose
> > > value is then returned to userspace via the EDAC 'threshold' sysfs
> > > attribute (small info leak and wrong reported threshold).
> > >
> > > Return 0 for indices outside the table.
> >
> > Why return 0 instead of errno?
> 
> The threshold count field is 3 bits wide, with encodings 0-2 and
> 6-7 reserved. ecs_supp_threshold[] is a sparse array that only
> initializes entries 3-5 (256/1024/4096), so when a device reports
> one of the reserved encodings 0-2, the current code already
> returns 0. The patch just extends the same treatment to the
> remaining reserved indices instead of reading past the table.
> 0 is also not a valid threshold value, so userspace can tell it
> apart from a real one.

Hi Yili,

I see the appeal of keeping this as the smallest possible fix, but is the
existing behavior for 0-2 something we intentionally want to preserve, or
just how the current implementation works?

Documentation/ABI/testing/sysfs-edac-ecs lists 256, 1024, and 4096 as the
threshold values. If 0 is meant to represent a reserved encoding, should
that be part of the documented ABI?

> 
> Returning an errno would additionally require open-coding a
> separate getter for this one attribute, as it goes through the
> shared CXL_ECS_GET_ATTR() macro, which cannot propagate errors.

Does CXL_ECS_GET_ATTR() itself prevent that? It already has an int return
path and can return errors from cxl_mem_ecs_get_attrbs().

Is the real limitation just the interface used by the cxl_get_ecs_*()
helpers? Would something like this work:


static int cxl_get_ecs_threshold(u8 log_cap, u16 config, u16 *threshold)
{
	u8 index = FIELD_GET(CXL_ECS_THRESHOLD_COUNT_MASK, config);

	if (index >= ARRAY_SIZE(ecs_supp_threshold) ||
	    !ecs_supp_threshold[index])
		return -EINVAL;

	*threshold = ecs_supp_threshold[index];
	return 0;
}

-- Alison

> 
> So I'd prefer to keep returning 0 here.
> 
> Thanks,
> Yili