I'm Seunghun Han and work at the Affiliated Institute of ETRI. I found
a bug related to improper buffer size calculation in crb_fixup_cmd_size
function.
When the TPM CRB regions are two or more, the crb_map_io function calls
crb_fixup_cmd_size twice to calculate command buffer size and response
buffer size. The purpose of crb_fixup_cmd_size function is to trust
the ACPI region information.
However, the function compares only io_res argument with start and size
arguments. It means the io_res argument is one of command buffer and
response buffer regions. It also means the other region is not calculated
correctly by the function because io_res argument doesn't cover all TPM
CRB regions.
To fix this bug, I change crb_check_resource function for storing all TPB
CRB regions to a list and use the list to calculate command buffer size
and response buffer size correctly.
Signed-off-by: Seunghun Han <kkamagui@xxxxxxxxx>
---
drivers/char/tpm/tpm_crb.c | 50 ++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..b0e94e02e5eb 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -442,6 +442,9 @@ static int crb_check_resource(struct acpi_resource *ares, void *data)
acpi_dev_resource_address_space(ares, &win)) {
*io_res = *res;
io_res->name = NULL;
+
+ /* Add this TPM CRB resource to the list */
+ return 0;
}
return 1;
@@ -471,20 +474,30 @@ static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
* region vs the registers. Trust the ACPI region. Such broken systems
* probably cannot send large TPM commands since the buffer will be truncated.
*/
-static u64 crb_fixup_cmd_size(struct device *dev, struct resource *io_res,
+static u64 crb_fixup_cmd_size(struct device *dev, struct list_head *resources,
u64 start, u64 size)
{
- if (io_res->start > start || io_res->end < start)
- return size;
+ struct resource_entry *pos;
+ struct resource *cur_res;
+
+ /* Check all TPM CRB resources with the start and size values */
+ resource_list_for_each_entry(pos, resources) {
+ cur_res = pos->res;
+
+ if (cur_res->start > start || cur_res->end < start)
+ continue;
- if (start + size - 1 <= io_res->end)
- return size;
+ if (start + size - 1 <= cur_res->end)
+ return size;
- dev_err(dev,
- FW_BUG "ACPI region does not cover the entire command/response buffer. %pr vs %llx %llx\n",
- io_res, start, size);
+ dev_err(dev,
+ FW_BUG "ACPI region does not cover the entire command/response buffer. %pr vs %llx %llx\n",
+ cur_res, start, size);
+
+ return cur_res->end - start + 1;
+ }
- return io_res->end - start + 1;
+ return size;
}
static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
@@ -506,16 +519,18 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
&io_res);
if (ret < 0)
return ret;
- acpi_dev_free_resource_list(&resources);
if (resource_type(&io_res) != IORESOURCE_MEM) {
dev_err(dev, FW_BUG "TPM2 ACPI table does not define a memory resource\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_early;
}
priv->iobase = devm_ioremap_resource(dev, &io_res);
- if (IS_ERR(priv->iobase))
- return PTR_ERR(priv->iobase);
+ if (IS_ERR(priv->iobase)) {
+ ret = PTR_ERR(priv->iobase);
+ goto out_early;
+ }
/* The ACPI IO region starts at the head area and continues to include
* the control area, as one nice sane region except for some older
@@ -532,7 +547,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
ret = __crb_request_locality(dev, priv, 0);
if (ret)
- return ret;
+ goto out_early;
priv->regs_t = crb_map_res(dev, priv, &io_res, buf->control_address,
sizeof(struct crb_regs_tail));
@@ -552,7 +567,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
pa_high = ioread32(&priv->regs_t->ctrl_cmd_pa_high);
pa_low = ioread32(&priv->regs_t->ctrl_cmd_pa_low);
cmd_pa = ((u64)pa_high << 32) | pa_low;
- cmd_size = crb_fixup_cmd_size(dev, &io_res, cmd_pa,
+ cmd_size = crb_fixup_cmd_size(dev, &resources, cmd_pa,
ioread32(&priv->regs_t->ctrl_cmd_size));
dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
@@ -566,7 +581,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
memcpy_fromio(&__rsp_pa, &priv->regs_t->ctrl_rsp_pa, 8);
rsp_pa = le64_to_cpu(__rsp_pa);
- rsp_size = crb_fixup_cmd_size(dev, &io_res, rsp_pa,
+ rsp_size = crb_fixup_cmd_size(dev, &resources, rsp_pa,
ioread32(&priv->regs_t->ctrl_rsp_size));
if (cmd_pa != rsp_pa) {
@@ -596,6 +611,9 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
__crb_relinquish_locality(dev, priv, 0);
+out_early:
+ acpi_dev_free_resource_list(&resources);
+
return ret;
}
--
2.21.0
