Re: [PATCH v3 04/16] cxl/pci: Handle excessive CDAT length

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 16 Feb 2023 11:26:16 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> On Tue, Feb 14, 2023 at 11:33:11AM +0000, Jonathan Cameron wrote:
> > On Fri, 10 Feb 2023 21:25:04 +0100 Lukas Wunner <lukas@wunner.de> wrote:  
> > > If the length in the CDAT header is larger than the concatenation of the
> > > header and all table entries, then the CDAT exposed to user space
> > > contains trailing null bytes.
> > > 
> > > Not every consumer may be able to handle that.  Per Postel's robustness
> > > principle, "be liberal in what you accept" and silently reduce the
> > > cached length to avoid exposing those null bytes.  
> [...]
> > Fair enough. I'd argue that we are papering over broken hardware if
> > we hit these conditions, so given we aren't aware of any (I hope)
> > not sure this is stable material.  Argument in favor of stable being
> > that if we do get broken hardware we don't want an ABI change when
> > we paper over the garbage... hmm.  
> 
> Type 0 is assigned for DSMAS structures.  So user space might believe
> there's an additional DSMAS in the CDAT.  It *could* detect that the
> length is bogus (it is 0 but should be 24), but what if it doesn't
> check that?  It seems way too dangerous to leave this loophole open,
> hence the stable designation.
Ok

> 
> Thanks,
> 
> Lukas
> 
> > > --- a/drivers/cxl/core/pci.c
> > > +++ b/drivers/cxl/core/pci.c
> > > @@ -582,6 +582,9 @@ static int cxl_cdat_read_table(struct device *dev,
> > >  		}
> > >  	} while (entry_handle != CXL_DOE_TABLE_ACCESS_LAST_ENTRY);
> > >  
> > > +	/* Length in CDAT header may exceed concatenation of CDAT entries */
> > > +	cdat->length -= length;
> > > +
> > >  	return 0;
> > >  }
> > >