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 <[email protected]> --- 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
