Re: [PATCH] cxl/port: Validate decoder nr_targets against supported ways

[<dan.j.williams@intel.com>]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> nr_targets is the number of downstream ports in a decoder's target
> list. It is limited to the supported interleave ways: 1, 2, 3, 4, 6,
> 8, 12, 16. The existing check only enforces nr_targets <=
> CXL_DECODER_MAX_INTERLEAVE (16), allowing invalid values, like 5, 7,
> etc, to pass through.
> 
> Add is_nr_targets_valid() to check for supported interleave ways using
> ways_to_eiw(). Use it before allocating a decoder to avoid alloc/free
> churn when nr_targets is already known to be invalid.
> 
> This was found by inspection and hardens the code.

Well, now seeing this and trying to answer the risk question in 1/ led
to me to reconsider whether this is a good idea and whether we have a
deeper confusion at play:

1/ "Hardening" needs to come with an actual risk claim. For
   cxl_switch_decoder_alloc() the values come from hardware or ACPI and if
   those are buggy all bets are off. ACPI is already protected by a
   eiw_to_ways() conversion prior to calling cxl_root_decoder_alloc(), and
   hardware is even more accurate than this
   approach because it considers interleave ways capability.

2/ nr_targets != ways. ways_to_eiw() is the wrong validity check for
   switch decoders, nr_targets is a topology limitation, not a
   interleave_ways limitation. As far as I can see nothing breaks if a
   decoder claims to support a target count that is not a valid
   interleave_ways number.

So it was useful to go through this to get that clarified, but the
takeaway from 1/ is "there is insufficient risk" and the takeaway from
2/ is "nothing breaks even if switch decoder target count is 'invalid'".
The valid interleave_ways is a separate region-only property.