| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1413995036-22497-4-git-send-email-jarkko.sakkinen@linux.intel.com>
Date: Wed, 22 Oct 2014 19:23:56 +0300
From: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To: Peter Huewe <peterhuewe@....de>, Ashley Lai <ashley@...leylai.com>,
Marcel Selhorst <tpmdd@...horst.net>
Cc: tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
linux-api@...r.kernel.org, josh.triplett@...el.com,
christophe.ricard@...il.com, jason.gunthorpe@...idianresearch.com,
Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Subject: [PATCH v1 3/3] tpm: fix multiple race conditions in tpm_ppi.c
Traversal of the ACPI device tree was not done right. It should lookup
PPI only under the ACPI device that it is associated. Otherwise, it could
match to a wrong PPI interface if there are two TPM devices in the device
tree.
Removed global ACPI handle and version string from tpm_ppi.c as this
is racy. Instead they should be associated with the chip.
Moved code just a tiny bit towards two-phase allocation to implement
fix for the PPI race conditions.
Added missing copyright platter to tpm_ppi.c.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
---
drivers/char/tpm/tpm-chip.c | 4 +-
drivers/char/tpm/tpm.h | 16 ++++--
drivers/char/tpm/tpm_ppi.c | 137 +++++++++++++++++++++++++++-----------------
drivers/char/tpm/tpm_tis.c | 15 +++--
4 files changed, 110 insertions(+), 62 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 4a29421..2446422 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -143,7 +143,7 @@ int tpm_chip_register(struct tpm_chip *chip)
if (rc)
goto del_misc;
- rc = tpm_add_ppi(&chip->dev->kobj);
+ rc = tpm_add_ppi(chip);
if (rc)
goto del_sysfs;
@@ -183,7 +183,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
synchronize_rcu();
tpm_sysfs_del_device(chip);
- tpm_remove_ppi(&chip->dev->kobj);
+ tpm_remove_ppi(chip);
tpm_bios_log_teardown(chip->bios_dir);
tpm_dev_del_device(chip);
}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index fa0974c..16fab4f 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -27,6 +27,7 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/tpm.h>
+#include <linux/acpi.h>
enum tpm_const {
TPM_MINOR = 224, /* officially assigned */
@@ -94,6 +95,8 @@ struct tpm_vendor_specific {
#define TPM_VID_WINBOND 0x1050
#define TPM_VID_STM 0x104A
+#define TPM_PPI_VERSION_LEN 3
+
struct tpm_chip {
struct device *dev; /* Device stuff */
const struct tpm_class_ops *ops;
@@ -109,6 +112,11 @@ struct tpm_chip {
struct dentry **bios_dir;
+#ifdef CONFIG_ACPI
+ acpi_handle acpi_dev_handle;
+ char ppi_version[TPM_PPI_VERSION_LEN + 1];
+#endif /* CONFIG_ACPI */
+
struct list_head list;
};
@@ -338,15 +346,15 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
#ifdef CONFIG_ACPI
-extern int tpm_add_ppi(struct kobject *);
-extern void tpm_remove_ppi(struct kobject *);
+extern int tpm_add_ppi(struct tpm_chip *chip);
+extern void tpm_remove_ppi(struct tpm_chip *chip);
#else
-static inline int tpm_add_ppi(struct kobject *parent)
+static inline int tpm_add_ppi(struct tpm_chip *chip)
{
return 0;
}
-static inline void tpm_remove_ppi(struct kobject *parent)
+static inline void tpm_remove_ppi(struct tpm_chip *chip)
{
}
#endif
diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index 61dcc80..f8b2326 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -1,3 +1,22 @@
+/*
+ * Copyright (C) 2012-2014 Intel Corporation
+ *
+ * Authors:
+ * Xiaoyan Zhang <xiaoyan.zhang@...el.com>
+ * Jiang Liu <jiang.liu@...ux.intel.com>
+ * Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
+ *
+ * This file contains implementation of the sysfs interface for PPI.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+
#include <linux/acpi.h>
#include "tpm.h"
@@ -22,45 +41,22 @@ static const u8 tpm_ppi_uuid[] = {
0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53
};
-static char tpm_ppi_version[PPI_VERSION_LEN + 1];
-static acpi_handle tpm_ppi_handle;
-
-static acpi_status ppi_callback(acpi_handle handle, u32 level, void *context,
- void **return_value)
-{
- union acpi_object *obj;
-
- if (!acpi_check_dsm(handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
- 1 << TPM_PPI_FN_VERSION))
- return AE_OK;
-
- /* Cache version string */
- obj = acpi_evaluate_dsm_typed(handle, tpm_ppi_uuid,
- TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
- NULL, ACPI_TYPE_STRING);
- if (obj) {
- strlcpy(tpm_ppi_version, obj->string.pointer,
- PPI_VERSION_LEN + 1);
- ACPI_FREE(obj);
- }
-
- *return_value = handle;
-
- return AE_CTRL_TERMINATE;
-}
-
static inline union acpi_object *
-tpm_eval_dsm(int func, acpi_object_type type, union acpi_object *argv4)
+tpm_eval_dsm(acpi_handle dev_handle, int func, acpi_object_type type,
+ union acpi_object *argv4)
{
- BUG_ON(!tpm_ppi_handle);
- return acpi_evaluate_dsm_typed(tpm_ppi_handle, tpm_ppi_uuid,
- TPM_PPI_REVISION_ID, func, argv4, type);
+ BUG_ON(!dev_handle);
+ return acpi_evaluate_dsm_typed(dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID,
+ func, argv4, type);
}
static ssize_t tpm_show_ppi_version(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return scnprintf(buf, PAGE_SIZE, "%s\n", tpm_ppi_version);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return scnprintf(buf, PAGE_SIZE, "%s\n", chip->ppi_version);
}
static ssize_t tpm_show_ppi_request(struct device *dev,
@@ -68,8 +64,10 @@ static ssize_t tpm_show_ppi_request(struct device *dev,
{
ssize_t size = -EINVAL;
union acpi_object *obj;
+ struct tpm_chip *chip = dev_get_drvdata(dev);
- obj = tpm_eval_dsm(TPM_PPI_FN_GETREQ, ACPI_TYPE_PACKAGE, NULL);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETREQ,
+ ACPI_TYPE_PACKAGE, NULL);
if (!obj)
return -ENXIO;
@@ -103,14 +101,15 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
int func = TPM_PPI_FN_SUBREQ;
union acpi_object *obj, tmp;
union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(1, &tmp);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
/*
* the function to submit TPM operation request to pre-os environment
* is updated with function index from SUBREQ to SUBREQ2 since PPI
* version 1.1
*/
- if (acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
- 1 << TPM_PPI_FN_SUBREQ2))
+ if (acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_SUBREQ2))
func = TPM_PPI_FN_SUBREQ2;
/*
@@ -119,7 +118,7 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
* string/package type. For PPI version 1.0 and 1.1, use buffer type
* for compatibility, and use package type since 1.2 according to spec.
*/
- if (strcmp(tpm_ppi_version, "1.2") < 0) {
+ if (strcmp(chip->ppi_version, "1.2") < 0) {
if (sscanf(buf, "%d", &req) != 1)
return -EINVAL;
argv4.type = ACPI_TYPE_BUFFER;
@@ -131,7 +130,8 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
return -EINVAL;
}
- obj = tpm_eval_dsm(func, ACPI_TYPE_INTEGER, &argv4);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, func, ACPI_TYPE_INTEGER,
+ &argv4);
if (!obj) {
return -ENXIO;
} else {
@@ -157,6 +157,7 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
.buffer.length = 0,
.buffer.pointer = NULL
};
+ struct tpm_chip *chip = dev_get_drvdata(dev);
static char *info[] = {
"None",
@@ -171,9 +172,10 @@ static ssize_t tpm_show_ppi_transition_action(struct device *dev,
* (e.g. Capella with PPI 1.0) need integer/string/buffer type, so for
* compatibility, define params[3].type as buffer, if PPI version < 1.2
*/
- if (strcmp(tpm_ppi_version, "1.2") < 0)
+ if (strcmp(chip->ppi_version, "1.2") < 0)
obj = &tmp;
- obj = tpm_eval_dsm(TPM_PPI_FN_GETACT, ACPI_TYPE_INTEGER, obj);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETACT,
+ ACPI_TYPE_INTEGER, obj);
if (!obj) {
return -ENXIO;
} else {
@@ -196,8 +198,10 @@ static ssize_t tpm_show_ppi_response(struct device *dev,
acpi_status status = -EINVAL;
union acpi_object *obj, *ret_obj;
u64 req, res;
+ struct tpm_chip *chip = dev_get_drvdata(dev);
- obj = tpm_eval_dsm(TPM_PPI_FN_GETRSP, ACPI_TYPE_PACKAGE, NULL);
+ obj = tpm_eval_dsm(chip->acpi_dev_handle, TPM_PPI_FN_GETRSP,
+ ACPI_TYPE_PACKAGE, NULL);
if (!obj)
return -ENXIO;
@@ -248,7 +252,8 @@ cleanup:
return status;
}
-static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
+static ssize_t show_ppi_operations(acpi_handle dev_handle, char *buf, u32 start,
+ u32 end)
{
int i;
u32 ret;
@@ -264,14 +269,15 @@ static ssize_t show_ppi_operations(char *buf, u32 start, u32 end)
"User not required",
};
- if (!acpi_check_dsm(tpm_ppi_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
+ if (!acpi_check_dsm(dev_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
1 << TPM_PPI_FN_GETOPR))
return -EPERM;
tmp.integer.type = ACPI_TYPE_INTEGER;
for (i = start; i <= end; i++) {
tmp.integer.value = i;
- obj = tpm_eval_dsm(TPM_PPI_FN_GETOPR, ACPI_TYPE_INTEGER, &argv);
+ obj = tpm_eval_dsm(dev_handle, TPM_PPI_FN_GETOPR,
+ ACPI_TYPE_INTEGER, &argv);
if (!obj) {
return -ENOMEM;
} else {
@@ -291,14 +297,20 @@ static ssize_t tpm_show_ppi_tcg_operations(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return show_ppi_operations(buf, 0, PPI_TPM_REQ_MAX);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return show_ppi_operations(chip->acpi_dev_handle, buf, 0,
+ PPI_TPM_REQ_MAX);
}
static ssize_t tpm_show_ppi_vs_operations(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return show_ppi_operations(buf, PPI_VS_REQ_START, PPI_VS_REQ_END);
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+
+ return show_ppi_operations(chip->acpi_dev_handle, buf, PPI_VS_REQ_START,
+ PPI_VS_REQ_END);
}
static DEVICE_ATTR(version, S_IRUGO, tpm_show_ppi_version, NULL);
@@ -323,16 +335,37 @@ static struct attribute_group ppi_attr_grp = {
.attrs = ppi_attrs
};
-int tpm_add_ppi(struct kobject *parent)
+int tpm_add_ppi(struct tpm_chip *chip)
{
- /* Cache TPM ACPI handle and version string */
- acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
- ppi_callback, NULL, NULL, &tpm_ppi_handle);
- return tpm_ppi_handle ? sysfs_create_group(parent, &ppi_attr_grp) : 0;
+ union acpi_object *obj;
+ struct kobject *parent = &chip->dev->kobj;
+
+ if (!chip->acpi_dev_handle)
+ return 0;
+
+ if (!acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_VERSION))
+ return 0;
+
+ /* Cache PPI version string. */
+ obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, tpm_ppi_uuid,
+ TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
+ NULL, ACPI_TYPE_STRING);
+ if (obj) {
+ strlcpy(chip->ppi_version, obj->string.pointer,
+ PPI_VERSION_LEN + 1);
+ ACPI_FREE(obj);
+ } else
+ return -ENOMEM;
+
+ return chip->acpi_dev_handle ?
+ sysfs_create_group(parent, &ppi_attr_grp) : 0;
}
-void tpm_remove_ppi(struct kobject *parent)
+void tpm_remove_ppi(struct tpm_chip *chip)
{
- if (tpm_ppi_handle)
+ struct kobject *parent = &chip->dev->kobj;
+
+ if (chip->ppi_version[0] != '\0')
sysfs_remove_group(parent, &ppi_attr_grp);
}
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index e2cb04d..c427da2 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -536,8 +536,9 @@ static void tpm_tis_remove(struct tpm_chip *chip)
release_locality(chip, chip->vendor.locality, 1);
}
-static int tpm_tis_init(struct device *dev, resource_size_t start,
- resource_size_t len, unsigned int irq)
+static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle,
+ resource_size_t start, resource_size_t len,
+ unsigned int irq)
{
u32 vendor, intfcaps, intmask;
int rc, i, irq_s, irq_e, probe;
@@ -547,6 +548,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
if (IS_ERR(chip))
return PTR_ERR(chip);
+ chip->acpi_dev_handle = acpi_dev_handle;
+
chip->vendor.iobase = devm_ioremap(dev, start, len);
if (!chip->vendor.iobase)
return -EIO;
@@ -777,6 +780,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
{
resource_size_t start, len;
unsigned int irq = 0;
+ acpi_handle acpi_dev_handle = 0;
start = pnp_mem_start(pnp_dev, 0);
len = pnp_mem_len(pnp_dev, 0);
@@ -789,7 +793,10 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev,
if (is_itpm(pnp_dev))
itpm = true;
- return tpm_tis_init(&pnp_dev->dev, start, len, irq);
+ if (pnp_acpi_device(pnp_dev))
+ acpi_dev_handle = pnp_acpi_device(pnp_dev)->handle;
+
+ return tpm_tis_init(&pnp_dev->dev, acpi_dev_handle, start, len, irq);
}
static struct pnp_device_id tpm_pnp_tbl[] = {
@@ -858,7 +865,7 @@ static int __init init_tis(void)
rc = PTR_ERR(pdev);
goto err_dev;
}
- rc = tpm_tis_init(&pdev->dev, TIS_MEM_BASE, TIS_MEM_LEN, 0);
+ rc = tpm_tis_init(&pdev->dev, 0, TIS_MEM_BASE, TIS_MEM_LEN, 0);
if (rc)
goto err_init;
return 0;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists