Re: [PATCH v3] cxl/edac: Fail ECS threshold read for reserved encodings

[Cheatham, Benjamin]

On 10/21/2026 10:20 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

Nit: Should probably spell out ECS here for people (like myself) who don't
know what it means.
> 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).
> 
> The threshold count field also has reserved encodings 0-2, which
> currently read as 0 from the sparse lookup table. That behavior is
> not intentional; it is just an artifact of the array's implicit
> zero-fill. The ABI documentation only lists 256, 1024 and 4096 as
> supported values, and the write side (cxl_set_ecs_threshold())
> rejects everything else, so a reserved encoding can only come from
> the device reporting it. Rather than documenting 0 as well, make the
> read fail for all reserved encodings and only ever report the
> documented values.
> 
> None of these encodings can be written through the sysfs write path,
> and no device is known to report them, so this is hardening against
> a device returning values not covered by the spec rather than a fix
> for known hardware.
> 
> Convert the cxl_get_ecs_*() helpers to return an int and write the
> attribute value through an output parameter (u32 * to match the
> macro), so that errors propagate through the shared
> CXL_ECS_GET_ATTR() macro to the sysfs read.
> 
> Suggested-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> Signed-off-by: Yili Zhang <zhangyili01@xxxxxxxxx>
> ---

I was going to make a suggestion about updating the documentation to mention
this new behavior, but noticed the other attributes in the ABI docs don't mention
reads of reserved values resulting in an error. The doc mentions that these attributes
are implemented by the device's driver (duh), so updating the general docs doesn't make
much sense.

So I'm going to take it a step further and ask if we should be failing the write at all?
It seems like other driver implementations probably don't, so it would be weird if just the
CXL driver did. I haven't looked for any other callers, so if CXL is the only user then
the docs should probably be updated so other implementers can follow the same convention.
If not, then it may be worth looking at the other call sites to update their behavior.

That may suck to do if multiple subsystems are involved, so if everyone else is fine with CXL being
different then I guess I am too :)

Thanks,
Ben