Re: [PATCH 2/3] cxl: Add checksum verification to CDAT from CXL

[Dave Jiang <dave.jiang@intel.com>]

On 5/5/23 12:15 PM, Davidlohr Bueso wrote:
> On Fri, 05 May 2023, Dave Jiang wrote:
> 
>> /**
>>  * read_cdat_data - Read the CDAT data on this port
>>  * @port: Port to read data from
>> @@ -571,9 +581,17 @@ void read_cdat_data(struct cxl_port *port)
>>         /* Don't leave table data allocated on error */
>>         devm_kfree(dev, cdat_table);
>>         dev_err(dev, "CDAT data read error\n");
>> +        return;
> 
> Should this be a separate fixlet?

Yeah I can split that out.
> 
>>     }
>>
>>     port->cdat.table = cdat_table + sizeof(__le32);
>> +    if (cdat_checksum(port->cdat.table, cdat_length)) {
>> +        /* Don't leave table data allocated on error */
>> +        devm_kfree(dev, cdat_table);
>> +        dev_err(dev, "CDAT data checksum error\n");
>> +        return;
>> +    }
>> +
>>     port->cdat.length = cdat_length;
> 
> Upon error, port->cdat should remain consistent (only be set after 
> passing the
> checksum. Ie, wouldn't the following be better?

yup. I'll update.

> 
>        cdat_table += sizeof(__le32);
>        if (cdat_checksum()) {
>           ...
>        }
>        port->cdat.table = cdat_table;
>        port->cdat.length = cdat_length;
> 
> 
> Thanks,
> Davidlohr