Re: [PATCH v4 1/2] cxl/hdm: Allow zero sized HDM decoders

["Dan Williams (nvidia)" <djbw@kernel.org>]

sashiko-bot@ wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Logic error in poison_by_decoder silently skips unmapped DPA poison collection for endpoints with a zero-sized first decoder.
> 
> Pre-existing issues:
> - [High] Stack buffer over-read in init_hdm_decoder leaks kernel stack memory to userspace.
> --
> 
> commit b5ef0cd71d8dea661d93a7fc863b0b6ecc394576
> Author: Richard Cheng <icheng@nvidia.com>
> 
> cxl/hdm: Allow zero sized HDM decoders
> 
> CXL r3.2 8.2.4.20.12 and 14.13.10 permit committing an HDM decoder with
> size 0. This patch allows init_hdm_decoder() to enumerate these decoders
> with their hardware LOCK state instead of aborting the port. It also
> modifies poison_by_decoder() to scan the unmapped DPA tail when a
> decoder has no DPA resource.
> 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 0c80b76a5f9b4..b61f51134551d 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> 
> [ ... ]
> 
> > @@ -1096,6 +1100,13 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> >  	if (!committed)
> >  		return 0;
> >  
> > +	/*
> > +	 * A committed zero-size decoder reserves no DPA. Leave port->hdm_end
> > +	 * untouched.
> > +	 */
> > +	if (size == 0)
> > +		return 0;
> > +
> >  	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);
> 
> [Severity: High]
> This is a pre-existing issue, but does the interleave ways configuration
> here allow reading out of bounds of the target_list union higher up in
> init_hdm_decoder()?
> 
> drivers/cxl/core/hdm.c:init_hdm_decoder() {
>     ...
>     union {
>         u64 value;
>         unsigned char target_id[8];
>     } target_list;
>     ...
>     if (!cxled) {
>         ...
>         for (i = 0; i < cxld->interleave_ways; i++)
>             cxld->target_map[i] = target_list.target_id[i];
>         return 0;
>     }
>     ...
> }
> 
> Since CXL 3.0 supports up to 16-way interleaving, if interleave_ways is
> greater than 8, will this loop read past the 8-byte target_list.target_id
> array and copy uninitialized kernel stack memory into target_map?

No, hardware should never return an IW setting > 8 for a switch port,
but that could be made more explicit in the code.