Dear Denis,
Thank you for your patch. In the summary, I’d use imperative mood:
tpm: Prevent local DOS …
Am 27.08.25 um 04:21 schrieb Denis Aleksandrov:
Reads on tpm/tpm0/ppi/*operations can become very long on
misconfigured systems. Reading the TPM is a blocking operation,
thus a user could effectively trigger a DOS.
Resolve this by caching the results and avoiding the blocking
operations after the first read.
If you could elaborate, how to test this, and in possible error cases,
how to debug this – for example, how to disable the cache–, that’d be great.
Reported-by: Jan Stancek <[email protected]>
Signed-off-by: Denis Aleksandrov <[email protected]>
---
Changes in v2:
- Replaced file permission change with a caching mechanism as
suggested by Jarkko.
drivers/char/tpm/tpm_ppi.c | 88 ++++++++++++++++++++++++++++----------
1 file changed, 65 insertions(+), 23 deletions(-)
diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index d53fce1c9d6f..e0212893748e 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -33,6 +33,21 @@ static const guid_t tpm_ppi_guid =
GUID_INIT(0x3DDDFAA6, 0x361B, 0x4EB4,
0xA4, 0x24, 0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53);
+static const char * const tpm_ppi_info[] = {
+ "Not implemented",
+ "BIOS only",
+ "Blocked for OS by BIOS",
Is this x86 specific? If not maybe use *system firmware*?
+ "User required",
+ "User not required",
+};
+
+/* A spinlock to protect access to the cache from concurrent reads */
+static DEFINE_SPINLOCK(tpm_ppi_lock);
+
+static u32 ppi_operations_cache[PPI_VS_REQ_END + 1];
+
+static bool ppi_cache_populated;
+
static bool tpm_ppi_req_has_parameter(u64 req)
{
return req == 23;
@@ -277,8 +292,7 @@ static ssize_t tpm_show_ppi_response(struct device *dev,
return status;
}
-static ssize_t show_ppi_operations(acpi_handle dev_handle, char *buf, u32 start,
- u32 end)
+static ssize_t cache_ppi_operations(acpi_handle dev_handle, char *buf)
{
int i;
u32 ret;
@@ -286,34 +300,22 @@ static ssize_t show_ppi_operations(acpi_handle
dev_handle, char *buf, u32 start,
union acpi_object *obj, tmp;
union acpi_object argv = ACPI_INIT_DSM_ARGV4(1, &tmp);
- static char *info[] = {
- "Not implemented",
- "BIOS only",
- "Blocked for OS by BIOS",
- "User required",
- "User not required",
- };
-
if (!acpi_check_dsm(dev_handle, &tpm_ppi_guid, TPM_PPI_REVISION_ID_1,
1 << TPM_PPI_FN_GETOPR))
return -EPERM;
tmp.integer.type = ACPI_TYPE_INTEGER;
- for (i = start; i <= end; i++) {
+ for (i = 0; i <= PPI_VS_REQ_END; i++) {
tmp.integer.value = i;
obj = tpm_eval_dsm(dev_handle, TPM_PPI_FN_GETOPR,
ACPI_TYPE_INTEGER, &argv,
TPM_PPI_REVISION_ID_1);
- if (!obj) {
+ if (!obj)
return -ENOMEM;
- } else {
- ret = obj->integer.value;
- ACPI_FREE(obj);
- }
- if (ret > 0 && ret < ARRAY_SIZE(info))
- len += sysfs_emit_at(buf, len, "%d %d: %s\n",
- i, ret, info[ret]);
+ ret = obj->integer.value;
+ ppi_operations_cache[i] = ret;
+ ACPI_FREE(obj);
}
return len;
@@ -323,20 +325,60 @@ static ssize_t tpm_show_ppi_tcg_operations(struct device
*dev,
struct device_attribute *attr,
char *buf)
{
+ int i;
+ ssize_t len = 0;
+ u32 ret;
struct tpm_chip *chip = to_tpm_chip(dev);
- return show_ppi_operations(chip->acpi_dev_handle, buf, 0,
- PPI_TPM_REQ_MAX);
+ spin_lock(&tpm_ppi_lock);
+ if (!ppi_cache_populated) {
+ len = cache_ppi_operations(chip->acpi_dev_handle, buf);
+
+ if (len < 0)
+ return len;
+
+ ppi_cache_populated = true;
+ }
+
+ for (i = 0; i <= PPI_TPM_REQ_MAX; i++) {
+ ret = ppi_operations_cache[i];
+ if (ret > 0 && ret < ARRAY_SIZE(tpm_ppi_info))
+ len += sysfs_emit_at(buf, len, "%d %d: %s\n",
+ i, ret,
tpm_ppi_info[ret]);
+ }
+ spin_unlock(&tpm_ppi_lock);
+
+ return len;
}
static ssize_t tpm_show_ppi_vs_operations(struct device *dev,
struct device_attribute *attr,
char *buf)
{
+ int i;
+ ssize_t len = 0;
+ u32 ret;
struct tpm_chip *chip = to_tpm_chip(dev);
- return show_ppi_operations(chip->acpi_dev_handle, buf, PPI_VS_REQ_START,
- PPI_VS_REQ_END);
+ spin_lock(&tpm_ppi_lock);
+ if (!ppi_cache_populated) {
+ len = cache_ppi_operations(chip->acpi_dev_handle, buf);
+
+ if (len < 0)
+ return len;
+
+ ppi_cache_populated = true;
+ }
+
+ for (i = PPI_VS_REQ_START; i <= PPI_VS_REQ_END; i++) {
+ ret = ppi_operations_cache[i];
+ if (ret > 0 && ret < ARRAY_SIZE(tpm_ppi_info))
+ len += sysfs_emit_at(buf, len, "%d %d: %s\n",
+ i, ret,
tpm_ppi_info[ret]);
+ }
+ spin_unlock(&tpm_ppi_lock);
+
+ return len;
}
static DEVICE_ATTR(version, S_IRUGO, tpm_show_ppi_version, NULL);
The diff looks good. Feel free to carry:
Reviewed-by: Paul Menzel <[email protected]>
Kind regards,
Paul Menzel
