On 4/24/23 3:01 PM, Dan Williams wrote:
Dave Jiang wrote:
A CDAT table is available from a CXL device. The table is read by the
driver and cached in software. With the CXL subsystem needing to parse the
CDAT table, the checksum should be verified. Add checksum verification
after the CDAT table is read from device.
Reviewed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
---
v3:
- Just return the final sum. (Alison)
v2:
- Drop ACPI checksum export and just use local verification. (Dan)
---
drivers/cxl/core/pci.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 25b7e8125d5d..9c7e2f69d9ca 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -528,6 +528,16 @@ static int cxl_cdat_read_table(struct device *dev,
return 0;
}
+static unsigned char cdat_checksum(void *buf, size_t size)
+{
+ unsigned char sum, *data = buf;
+ size_t i;
+
+ for (sum = 0, i = 0; i < size; i++)
+ sum += data[i];
+ return sum;
+}
+
/**
* read_cdat_data - Read the CDAT data on this port
* @port: Port to read data from
@@ -573,6 +583,12 @@ void read_cdat_data(struct cxl_port *port)
}
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");
+ }
+
port->cdat.length = cdat_length;
I think read_cdat_data() is confused about error cases. I note that
/sys/firmware/acpi/tables does not emit the entry if the table has bad
length or bad checksum. If you want to have a debug mode then maybe make
it a compile time option, but I otherwise do not see the benefit of
publishing known bad tables to userspace.
I'll have it return on errors.
