2017-10-26 13:55:04

by Jarkko Sakkinen

Subject: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

Device number (the character device index) is not a stable identifier
for a TPM chip. That is the reason why every call site passes
TPM_ANY_NUM to tpm_chip_find_get().

This commit changes the API in a way that instead a struct tpm_chip
instance is given and NULL means the default chip. In addition, this
commit refines the documentation to be up to date with the
implementation.

Suggested-by: Jason Gunthorpe <[email protected]> (@chip_num -> @chip)
Signed-off-by: Jarkko Sakkinen <[email protected]>
---
v3:
* If chip != NULL was given to tpm_chip_find_get(), it tried to reserve chip
associated with the device number 0 instead of trying to reserve the given
chip instance.
v2:
* Further defined function documentation.
* Changed @chip_num to @chip instead of removing the parameter as suggested by
Jason Gunthorpe.
drivers/char/hw_random/tpm-rng.c | 2 +-
drivers/char/tpm/tpm-chip.c | 24 ++++---
drivers/char/tpm/tpm-interface.c | 135 +++++++++++++++++++-----------------
drivers/char/tpm/tpm.h | 2 +-
include/linux/tpm.h | 38 +++++-----
security/integrity/ima/ima_crypto.c | 2 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_queue.c | 2 +-
security/keys/trusted.c | 35 +++++-----
9 files changed, 126 insertions(+), 116 deletions(-)

diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
index d6d448266f07..c5e363825af0 100644
--- a/drivers/char/hw_random/tpm-rng.c
+++ b/drivers/char/hw_random/tpm-rng.c
@@ -25,7 +25,7 @@

static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
{
- return tpm_get_random(TPM_ANY_NUM, data, max);
+ return tpm_get_random(NULL, data, max);
}

static struct hwrng tpm_rng = {
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index a114e8f7fb90..bab9c14e040c 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
EXPORT_SYMBOL_GPL(tpm_put_ops);

/**
- * tpm_chip_find_get() - return tpm_chip for a given chip number
- * @chip_num: id to find
+ * tpm_chip_find_get() - find and reserve a TPM chip
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
*
- * The return'd chip has been tpm_try_get_ops'd and must be released via
- * tpm_put_ops
+ * Finds a TPM chip and reserves its class device and operations. The chip must
+ * be released with tpm_chip_put_ops() after use.
+ *
+ * Return:
+ * A reserved &struct tpm_chip instance.
+ * %NULL if a chip is not found.
+ * %NULL if the chip is not available.
*/
-struct tpm_chip *tpm_chip_find_get(int chip_num)
+struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
{
- struct tpm_chip *chip, *res = NULL;
+ struct tpm_chip *res = NULL;
+ int chip_num = 0;
int chip_prev;

mutex_lock(&idr_lock);

- if (chip_num == TPM_ANY_NUM) {
- chip_num = 0;
+ if (!chip) {
do {
chip_prev = chip_num;
chip = idr_get_next(&dev_nums_idr, &chip_num);
@@ -105,8 +110,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
}
} while (chip_prev != chip_num);
} else {
- chip = idr_find(&dev_nums_idr, chip_num);
- if (chip && !tpm_try_get_ops(chip))
+ if (!tpm_try_get_ops(chip))
res = chip;
}

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ebe0a1d36d8c..19f820f775b5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
}

/**
- * tpm_is_tpm2 - is the chip a TPM2 chip?
- * @chip_num: tpm idx # or ANY
+ * tpm_is_tpm2 - do we a have a TPM2 chip?
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
*
- * Returns < 0 on error, and 1 or 0 on success depending whether the chip
- * is a TPM2 chip.
+ * Return:
+ * 1 if we have a TPM2 chip.
+ * 0 if we don't have a TPM2 chip.
+ * A negative number for system errors (errno).
*/
-int tpm_is_tpm2(u32 chip_num)
+int tpm_is_tpm2(struct tpm_chip *chip)
{
- struct tpm_chip *chip;
int rc;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL)
+ chip = tpm_chip_find_get(chip);
+ if (!chip)
return -ENODEV;

rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
@@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
EXPORT_SYMBOL_GPL(tpm_is_tpm2);

/**
- * tpm_pcr_read - read a pcr value
- * @chip_num: tpm idx # or ANY
- * @pcr_idx: pcr idx to retrieve
- * @res_buf: TPM_PCR value
- * size of res_buf is 20 bytes (or NULL if you don't care)
+ * tpm_pcr_read - read a PCR value from SHA1 bank
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @pcr_idx: the PCR to be retrieved
+ * @res_buf: the value of the PCR
*
- * The TPM driver should be built-in, but for whatever reason it
- * isn't, protect against the chip disappearing, by incrementing
- * the module usage count.
+ * Return: same as with tpm_transmit_cmd()
*/
-int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
+int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
{
- struct tpm_chip *chip;
int rc;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL)
+ chip = tpm_chip_find_get(chip);
+ if (!chip)
return -ENODEV;
if (chip->flags & TPM_CHIP_FLAG_TPM2)
rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
@@ -889,25 +886,26 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
}

/**
- * tpm_pcr_extend - extend pcr value with hash
- * @chip_num: tpm idx # or AN&
- * @pcr_idx: pcr idx to extend
- * @hash: hash value used to extend pcr value
+ * tpm_pcr_extend - extend a PCR value in SHA1 bank.
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @pcr_idx: the PCR to be retrieved
+ * @hash: the hash value used to extend the PCR value
*
- * The TPM driver should be built-in, but for whatever reason it
- * isn't, protect against the chip disappearing, by incrementing
- * the module usage count.
+ * Note: with TPM 2.0 extends also those banks with a known digest size to the
+ * cryto subsystem in order to prevent malicious use of those PCR banks. In the
+ * future we should dynamically determine digest sizes.
+ *
+ * Return: same as with tpm_transmit_cmd()
*/
-int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
+int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
{
int rc;
- struct tpm_chip *chip;
struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
u32 count = 0;
int i;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL)
+ chip = tpm_chip_find_get(chip);
+ if (!chip)
return -ENODEV;

if (chip->flags & TPM_CHIP_FLAG_TPM2) {
@@ -1019,17 +1017,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
return rc;
}

-int tpm_send(u32 chip_num, void *cmd, size_t buflen)
+/**
+ * tpm_send - send a TPM command
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @cmd: a TPM command buffer
+ * @buflen: the length of the TPM command buffer
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
{
- struct tpm_chip *chip;
int rc;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL)
+ chip = tpm_chip_find_get(chip);
+ if (!chip)
return -ENODEV;

rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
- "attempting tpm_cmd");
+ "attempting to a send a command");
tpm_put_ops(chip);
return rc;
}
@@ -1127,16 +1132,15 @@ static const struct tpm_input_header tpm_getrandom_header = {
};

/**
- * tpm_get_random() - Get random bytes from the tpm's RNG
- * @chip_num: A specific chip number for the request or TPM_ANY_NUM
- * @out: destination buffer for the random bytes
- * @max: the max number of bytes to write to @out
+ * tpm_get_random() - get random bytes from the TPM's RNG
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @out: destination buffer for the random bytes
+ * @max: the max number of bytes to write to @out
*
- * Returns < 0 on error and the number of bytes read on success
+ * Return: same as with tpm_transmit_cmd()
*/
-int tpm_get_random(u32 chip_num, u8 *out, size_t max)
+int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
{
- struct tpm_chip *chip;
struct tpm_cmd_t tpm_cmd;
u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
int err, total = 0, retries = 5;
@@ -1145,8 +1149,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
return -EINVAL;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL)
+ chip = tpm_chip_find_get(chip);
+ if (!chip)
return -ENODEV;

if (chip->flags & TPM_CHIP_FLAG_TPM2) {
@@ -1188,22 +1192,23 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
EXPORT_SYMBOL_GPL(tpm_get_random);

/**
- * tpm_seal_trusted() - seal a trusted key
- * @chip_num: A specific chip number for the request or TPM_ANY_NUM
- * @options: authentication values and other options
- * @payload: the key data in clear and encrypted form
+ * tpm_seal_trusted() - seal a trusted key payload
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @options: authentication values and other options
+ * @payload: the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
*
- * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
- * are supported.
+ * Return: same as with tpm_transmit_cmd()
*/
-int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
+int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
- struct tpm_chip *chip;
int rc;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ chip = tpm_chip_find_get(chip);
+ if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
return -ENODEV;

rc = tpm2_seal_trusted(chip, payload, options);
@@ -1215,21 +1220,23 @@ EXPORT_SYMBOL_GPL(tpm_seal_trusted);

/**
* tpm_unseal_trusted() - unseal a trusted key
- * @chip_num: A specific chip number for the request or TPM_ANY_NUM
- * @options: authentication values and other options
- * @payload: the key data in clear and encrypted form
+ * @chip: a &struct tpm_chip instance, %NULL for the default chip
+ * @options: authentication values and other options
+ * @payload: the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
*
- * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
- * are supported.
+ * Return: same as with tpm_transmit_cmd()
*/
-int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
+int tpm_unseal_trusted(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
- struct tpm_chip *chip;
int rc;

- chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ chip = tpm_chip_find_get(chip);
+ if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
return -ENODEV;

rc = tpm2_unseal_trusted(chip, payload, options);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index c1866cc02e30..6c189174c0d3 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -516,7 +516,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
delay_msec * 1000);
};

-struct tpm_chip *tpm_chip_find_get(int chip_num);
+struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip);
__must_check int tpm_try_get_ops(struct tpm_chip *chip);
void tpm_put_ops(struct tpm_chip *chip);

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 5a090f5ab335..ddc9b88ff6d3 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -24,11 +24,6 @@

#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */

-/*
- * Chip num is this value or a valid tpm idx
- */
-#define TPM_ANY_NUM 0xFFFF
-
struct tpm_chip;
struct trusted_key_payload;
struct trusted_key_options;
@@ -54,42 +49,47 @@ struct tpm_class_ops {

#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)

-extern int tpm_is_tpm2(u32 chip_num);
-extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
-extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
-extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
-extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
-extern int tpm_seal_trusted(u32 chip_num,
+extern int tpm_is_tpm2(struct tpm_chip *chip);
+extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
+extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
+extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
+extern int tpm_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options);
-extern int tpm_unseal_trusted(u32 chip_num,
+extern int tpm_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options);
#else
-static inline int tpm_is_tpm2(u32 chip_num)
+static inline int tpm_is_tpm2(struct tpm_chip *chip)
{
return -ENODEV;
}
-static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
+static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+{
return -ENODEV;
}
-static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
+static inline int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx,
+ const u8 *hash)
+{
return -ENODEV;
}
-static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
+static inline int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
+{
return -ENODEV;
}
-static inline int tpm_get_random(u32 chip_num, u8 *data, size_t max) {
+static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
+{
return -ENODEV;
}

-static inline int tpm_seal_trusted(u32 chip_num,
+static inline int tpm_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
return -ENODEV;
}
-static inline int tpm_unseal_trusted(u32 chip_num,
+static inline int tpm_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options)
{
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 802d5d20f36f..3371d134ee62 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -644,7 +644,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
if (!ima_used_chip)
return;

- if (tpm_pcr_read(TPM_ANY_NUM, idx, pcr) != 0)
+ if (tpm_pcr_read(NULL, idx, pcr) != 0)
pr_err("Error Communicating to TPM chip\n");
}

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 2967d497a665..29b72cd2502e 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -110,7 +110,7 @@ int __init ima_init(void)
int rc;

ima_used_chip = 0;
- rc = tpm_pcr_read(TPM_ANY_NUM, 0, pcr_i);
+ rc = tpm_pcr_read(NULL, 0, pcr_i);
if (rc == 0)
ima_used_chip = 1;

diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index a02a86d51102..418f35e38015 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -145,7 +145,7 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
if (!ima_used_chip)
return result;

- result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
+ result = tpm_pcr_extend(NULL, pcr, hash);
if (result != 0)
pr_err("Error Communicating to TPM chip, result: %d\n", result);
return result;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ddfaebf60fc8..d2b20b1cc2f1 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -355,13 +355,12 @@ static int TSS_checkhmac2(unsigned char *buffer,
* For key specific tpm requests, we will generate and send our
* own TPM command packets using the drivers send function.
*/
-static int trusted_tpm_send(const u32 chip_num, unsigned char *cmd,
- size_t buflen)
+static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
{
int rc;

dump_tpm_buf(cmd);
- rc = tpm_send(chip_num, cmd, buflen);
+ rc = tpm_send(NULL, cmd, buflen);
dump_tpm_buf(cmd);
if (rc > 0)
/* Can't return positive return codes values to keyctl */
@@ -382,10 +381,10 @@ static int pcrlock(const int pcrnum)

if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
+ ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
if (ret != SHA1_DIGEST_SIZE)
return ret;
- return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
+ return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
}

/*
@@ -398,7 +397,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
unsigned char ononce[TPM_NONCE_SIZE];
int ret;

- ret = tpm_get_random(TPM_ANY_NUM, ononce, TPM_NONCE_SIZE);
+ ret = tpm_get_random(NULL, ononce, TPM_NONCE_SIZE);
if (ret != TPM_NONCE_SIZE)
return ret;

@@ -410,7 +409,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
store32(tb, handle);
storebytes(tb, ononce, TPM_NONCE_SIZE);

- ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
+ ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
if (ret < 0)
return ret;

@@ -434,7 +433,7 @@ static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
store16(tb, TPM_TAG_RQU_COMMAND);
store32(tb, TPM_OIAP_SIZE);
store32(tb, TPM_ORD_OIAP);
- ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
+ ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
if (ret < 0)
return ret;

@@ -493,7 +492,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
if (ret < 0)
goto out;

- ret = tpm_get_random(TPM_ANY_NUM, td->nonceodd, TPM_NONCE_SIZE);
+ ret = tpm_get_random(NULL, td->nonceodd, TPM_NONCE_SIZE);
if (ret != TPM_NONCE_SIZE)
goto out;
ordinal = htonl(TPM_ORD_SEAL);
@@ -542,7 +541,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
store8(tb, cont);
storebytes(tb, td->pubauth, SHA1_DIGEST_SIZE);

- ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
+ ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
if (ret < 0)
goto out;

@@ -603,7 +602,7 @@ static int tpm_unseal(struct tpm_buf *tb,

ordinal = htonl(TPM_ORD_UNSEAL);
keyhndl = htonl(SRKHANDLE);
- ret = tpm_get_random(TPM_ANY_NUM, nonceodd, TPM_NONCE_SIZE);
+ ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE);
if (ret != TPM_NONCE_SIZE) {
pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
return ret;
@@ -635,7 +634,7 @@ static int tpm_unseal(struct tpm_buf *tb,
store8(tb, cont);
storebytes(tb, authdata2, SHA1_DIGEST_SIZE);

- ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
+ ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
if (ret < 0) {
pr_info("trusted_key: authhmac failed (%d)\n", ret);
return ret;
@@ -748,7 +747,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
int i;
int tpm2;

- tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
+ tpm2 = tpm_is_tpm2(NULL);
if (tpm2 < 0)
return tpm2;

@@ -917,7 +916,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
struct trusted_key_options *options;
int tpm2;

- tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
+ tpm2 = tpm_is_tpm2(NULL);
if (tpm2 < 0)
return NULL;

@@ -967,7 +966,7 @@ static int trusted_instantiate(struct key *key,
size_t key_len;
int tpm2;

- tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
+ tpm2 = tpm_is_tpm2(NULL);
if (tpm2 < 0)
return tpm2;

@@ -1008,7 +1007,7 @@ static int trusted_instantiate(struct key *key,
switch (key_cmd) {
case Opt_load:
if (tpm2)
- ret = tpm_unseal_trusted(TPM_ANY_NUM, payload, options);
+ ret = tpm_unseal_trusted(NULL, payload, options);
else
ret = key_unseal(payload, options);
dump_payload(payload);
@@ -1018,13 +1017,13 @@ static int trusted_instantiate(struct key *key,
break;
case Opt_new:
key_len = payload->key_len;
- ret = tpm_get_random(TPM_ANY_NUM, payload->key, key_len);
+ ret = tpm_get_random(NULL, payload->key, key_len);
if (ret != key_len) {
pr_info("trusted_key: key_create failed (%d)\n", ret);
goto out;
}
if (tpm2)
- ret = tpm_seal_trusted(TPM_ANY_NUM, payload, options);
+ ret = tpm_seal_trusted(NULL, payload, options);
else
ret = key_seal(payload, options);
if (ret < 0)
--
2.14.1

2017-10-26 14:08:41

by Jarkko Sakkinen

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Thu, Oct 26, 2017 at 03:54:50PM +0200, Jarkko Sakkinen wrote:
> Device number (the character device index) is not a stable identifier
> for a TPM chip. That is the reason why every call site passes
> TPM_ANY_NUM to tpm_chip_find_get().
>
> This commit changes the API in a way that instead a struct tpm_chip
> instance is given and NULL means the default chip. In addition, this
> commit refines the documentation to be up to date with the
> implementation.
>
> Suggested-by: Jason Gunthorpe <[email protected]> (@chip_num -> @chip)
> Signed-off-by: Jarkko Sakkinen <[email protected]>

Jason, once you have given this reviewed-by can you do a patch set that
includes this and your and PrasannaKumar's change? It could be a while
before keyring and IMA maintainers have reviewed my change.

/Jarkko

2017-10-26 14:10:49

by PrasannaKumar Muralidharan

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

Hi Jarkko,

On 26 October 2017 at 19:24, Jarkko Sakkinen
<[email protected]> wrote:
> Device number (the character device index) is not a stable identifier
> for a TPM chip. That is the reason why every call site passes
> TPM_ANY_NUM to tpm_chip_find_get().
>
> This commit changes the API in a way that instead a struct tpm_chip
> instance is given and NULL means the default chip. In addition, this
> commit refines the documentation to be up to date with the
> implementation.
>
> Suggested-by: Jason Gunthorpe <[email protected]> (@chip_num -> @chip)
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> v3:
> * If chip != NULL was given to tpm_chip_find_get(), it tried to reserve chip
> associated with the device number 0 instead of trying to reserve the given
> chip instance.
> v2:
> * Further defined function documentation.
> * Changed @chip_num to @chip instead of removing the parameter as suggested by
> Jason Gunthorpe.
> drivers/char/hw_random/tpm-rng.c | 2 +-
> drivers/char/tpm/tpm-chip.c | 24 ++++---
> drivers/char/tpm/tpm-interface.c | 135 +++++++++++++++++++-----------------
> drivers/char/tpm/tpm.h | 2 +-
> include/linux/tpm.h | 38 +++++-----
> security/integrity/ima/ima_crypto.c | 2 +-
> security/integrity/ima/ima_init.c | 2 +-
> security/integrity/ima/ima_queue.c | 2 +-
> security/keys/trusted.c | 35 +++++-----
> 9 files changed, 126 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
> index d6d448266f07..c5e363825af0 100644
> --- a/drivers/char/hw_random/tpm-rng.c
> +++ b/drivers/char/hw_random/tpm-rng.c
> @@ -25,7 +25,7 @@
>
> static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> {
> - return tpm_get_random(TPM_ANY_NUM, data, max);
> + return tpm_get_random(NULL, data, max);
> }
>
> static struct hwrng tpm_rng = {
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index a114e8f7fb90..bab9c14e040c 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
> EXPORT_SYMBOL_GPL(tpm_put_ops);
>
> /**
> - * tpm_chip_find_get() - return tpm_chip for a given chip number
> - * @chip_num: id to find
> + * tpm_chip_find_get() - find and reserve a TPM chip
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> *
> - * The return'd chip has been tpm_try_get_ops'd and must be released via
> - * tpm_put_ops
> + * Finds a TPM chip and reserves its class device and operations. The chip must
> + * be released with tpm_chip_put_ops() after use.
> + *
> + * Return:
> + * A reserved &struct tpm_chip instance.
> + * %NULL if a chip is not found.
> + * %NULL if the chip is not available.
> */
> -struct tpm_chip *tpm_chip_find_get(int chip_num)
> +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
> {
> - struct tpm_chip *chip, *res = NULL;
> + struct tpm_chip *res = NULL;
> + int chip_num = 0;
> int chip_prev;
>
> mutex_lock(&idr_lock);
>
> - if (chip_num == TPM_ANY_NUM) {
> - chip_num = 0;
> + if (!chip) {
> do {
> chip_prev = chip_num;
> chip = idr_get_next(&dev_nums_idr, &chip_num);
> @@ -105,8 +110,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
> }
> } while (chip_prev != chip_num);
> } else {
> - chip = idr_find(&dev_nums_idr, chip_num);
> - if (chip && !tpm_try_get_ops(chip))
> + if (!tpm_try_get_ops(chip))
> res = chip;
> }
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index ebe0a1d36d8c..19f820f775b5 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> }
>
> /**
> - * tpm_is_tpm2 - is the chip a TPM2 chip?
> - * @chip_num: tpm idx # or ANY
> + * tpm_is_tpm2 - do we a have a TPM2 chip?
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> *
> - * Returns < 0 on error, and 1 or 0 on success depending whether the chip
> - * is a TPM2 chip.
> + * Return:
> + * 1 if we have a TPM2 chip.
> + * 0 if we don't have a TPM2 chip.
> + * A negative number for system errors (errno).
> */
> -int tpm_is_tpm2(u32 chip_num)
> +int tpm_is_tpm2(struct tpm_chip *chip)
> {
> - struct tpm_chip *chip;
> int rc;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL)
> + chip = tpm_chip_find_get(chip);
> + if (!chip)
> return -ENODEV;
>
> rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
> @@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
> EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>
> /**
> - * tpm_pcr_read - read a pcr value
> - * @chip_num: tpm idx # or ANY
> - * @pcr_idx: pcr idx to retrieve
> - * @res_buf: TPM_PCR value
> - * size of res_buf is 20 bytes (or NULL if you don't care)
> + * tpm_pcr_read - read a PCR value from SHA1 bank
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @pcr_idx: the PCR to be retrieved
> + * @res_buf: the value of the PCR
> *
> - * The TPM driver should be built-in, but for whatever reason it
> - * isn't, protect against the chip disappearing, by incrementing
> - * the module usage count.
> + * Return: same as with tpm_transmit_cmd()
> */
> -int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
> +int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> {
> - struct tpm_chip *chip;
> int rc;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL)
> + chip = tpm_chip_find_get(chip);
> + if (!chip)
> return -ENODEV;
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
> @@ -889,25 +886,26 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
> }
>
> /**
> - * tpm_pcr_extend - extend pcr value with hash
> - * @chip_num: tpm idx # or AN&
> - * @pcr_idx: pcr idx to extend
> - * @hash: hash value used to extend pcr value
> + * tpm_pcr_extend - extend a PCR value in SHA1 bank.
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @pcr_idx: the PCR to be retrieved
> + * @hash: the hash value used to extend the PCR value
> *
> - * The TPM driver should be built-in, but for whatever reason it
> - * isn't, protect against the chip disappearing, by incrementing
> - * the module usage count.
> + * Note: with TPM 2.0 extends also those banks with a known digest size to the
> + * cryto subsystem in order to prevent malicious use of those PCR banks. In the
> + * future we should dynamically determine digest sizes.
> + *
> + * Return: same as with tpm_transmit_cmd()
> */
> -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
> {
> int rc;
> - struct tpm_chip *chip;
> struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> u32 count = 0;
> int i;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL)
> + chip = tpm_chip_find_get(chip);
> + if (!chip)
> return -ENODEV;
>
> if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> @@ -1019,17 +1017,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
> return rc;
> }
>
> -int tpm_send(u32 chip_num, void *cmd, size_t buflen)
> +/**
> + * tpm_send - send a TPM command
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @cmd: a TPM command buffer
> + * @buflen: the length of the TPM command buffer
> + *
> + * Return: same as with tpm_transmit_cmd()
> + */
> +int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> {
> - struct tpm_chip *chip;
> int rc;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL)
> + chip = tpm_chip_find_get(chip);
> + if (!chip)
> return -ENODEV;
>
> rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
> - "attempting tpm_cmd");
> + "attempting to a send a command");
> tpm_put_ops(chip);
> return rc;
> }
> @@ -1127,16 +1132,15 @@ static const struct tpm_input_header tpm_getrandom_header = {
> };
>
> /**
> - * tpm_get_random() - Get random bytes from the tpm's RNG
> - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> - * @out: destination buffer for the random bytes
> - * @max: the max number of bytes to write to @out
> + * tpm_get_random() - get random bytes from the TPM's RNG
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @out: destination buffer for the random bytes
> + * @max: the max number of bytes to write to @out
> *
> - * Returns < 0 on error and the number of bytes read on success
> + * Return: same as with tpm_transmit_cmd()
> */
> -int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> +int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> {
> - struct tpm_chip *chip;
> struct tpm_cmd_t tpm_cmd;
> u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
> int err, total = 0, retries = 5;
> @@ -1145,8 +1149,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
> return -EINVAL;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL)
> + chip = tpm_chip_find_get(chip);
> + if (!chip)
> return -ENODEV;
>
> if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> @@ -1188,22 +1192,23 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> EXPORT_SYMBOL_GPL(tpm_get_random);
>
> /**
> - * tpm_seal_trusted() - seal a trusted key
> - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> - * @options: authentication values and other options
> - * @payload: the key data in clear and encrypted form
> + * tpm_seal_trusted() - seal a trusted key payload
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @options: authentication values and other options
> + * @payload: the key data in clear and encrypted form
> + *
> + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
> + * the keyring subsystem.
> *
> - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
> - * are supported.
> + * Return: same as with tpm_transmit_cmd()
> */
> -int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
> +int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
> struct trusted_key_options *options)
> {
> - struct tpm_chip *chip;
> int rc;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> + chip = tpm_chip_find_get(chip);
> + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> return -ENODEV;
>
> rc = tpm2_seal_trusted(chip, payload, options);
> @@ -1215,21 +1220,23 @@ EXPORT_SYMBOL_GPL(tpm_seal_trusted);
>
> /**
> * tpm_unseal_trusted() - unseal a trusted key
> - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> - * @options: authentication values and other options
> - * @payload: the key data in clear and encrypted form
> + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> + * @options: authentication values and other options
> + * @payload: the key data in clear and encrypted form
> + *
> + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
> + * the keyring subsystem.
> *
> - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
> - * are supported.
> + * Return: same as with tpm_transmit_cmd()
> */
> -int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
> +int tpm_unseal_trusted(struct tpm_chip *chip,
> + struct trusted_key_payload *payload,
> struct trusted_key_options *options)
> {
> - struct tpm_chip *chip;
> int rc;
>
> - chip = tpm_chip_find_get(chip_num);
> - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> + chip = tpm_chip_find_get(chip);
> + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> return -ENODEV;
>
> rc = tpm2_unseal_trusted(chip, payload, options);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index c1866cc02e30..6c189174c0d3 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -516,7 +516,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
> delay_msec * 1000);
> };
>
> -struct tpm_chip *tpm_chip_find_get(int chip_num);
> +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip);
> __must_check int tpm_try_get_ops(struct tpm_chip *chip);
> void tpm_put_ops(struct tpm_chip *chip);
>
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 5a090f5ab335..ddc9b88ff6d3 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -24,11 +24,6 @@
>
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>
> -/*
> - * Chip num is this value or a valid tpm idx
> - */
> -#define TPM_ANY_NUM 0xFFFF
> -
> struct tpm_chip;
> struct trusted_key_payload;
> struct trusted_key_options;
> @@ -54,42 +49,47 @@ struct tpm_class_ops {
>
> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>
> -extern int tpm_is_tpm2(u32 chip_num);
> -extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
> -extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> -extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
> -extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
> -extern int tpm_seal_trusted(u32 chip_num,
> +extern int tpm_is_tpm2(struct tpm_chip *chip);
> +extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> +extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> +extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
> +extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
> +extern int tpm_seal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options);
> -extern int tpm_unseal_trusted(u32 chip_num,
> +extern int tpm_unseal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options);
> #else
> -static inline int tpm_is_tpm2(u32 chip_num)
> +static inline int tpm_is_tpm2(struct tpm_chip *chip)
> {
> return -ENODEV;
> }
> -static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
> +static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +{
> return -ENODEV;
> }
> -static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
> +static inline int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx,
> + const u8 *hash)
> +{
> return -ENODEV;
> }
> -static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
> +static inline int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> +{
> return -ENODEV;
> }
> -static inline int tpm_get_random(u32 chip_num, u8 *data, size_t max) {
> +static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
> +{
> return -ENODEV;
> }
>
> -static inline int tpm_seal_trusted(u32 chip_num,
> +static inline int tpm_seal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options)
> {
> return -ENODEV;
> }
> -static inline int tpm_unseal_trusted(u32 chip_num,
> +static inline int tpm_unseal_trusted(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options)
> {
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 802d5d20f36f..3371d134ee62 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -644,7 +644,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
> if (!ima_used_chip)
> return;
>
> - if (tpm_pcr_read(TPM_ANY_NUM, idx, pcr) != 0)
> + if (tpm_pcr_read(NULL, idx, pcr) != 0)
> pr_err("Error Communicating to TPM chip\n");
> }
>
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 2967d497a665..29b72cd2502e 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -110,7 +110,7 @@ int __init ima_init(void)
> int rc;
>
> ima_used_chip = 0;
> - rc = tpm_pcr_read(TPM_ANY_NUM, 0, pcr_i);
> + rc = tpm_pcr_read(NULL, 0, pcr_i);
> if (rc == 0)
> ima_used_chip = 1;
>
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index a02a86d51102..418f35e38015 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -145,7 +145,7 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
> if (!ima_used_chip)
> return result;
>
> - result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
> + result = tpm_pcr_extend(NULL, pcr, hash);
> if (result != 0)
> pr_err("Error Communicating to TPM chip, result: %d\n", result);
> return result;
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index ddfaebf60fc8..d2b20b1cc2f1 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -355,13 +355,12 @@ static int TSS_checkhmac2(unsigned char *buffer,
> * For key specific tpm requests, we will generate and send our
> * own TPM command packets using the drivers send function.
> */
> -static int trusted_tpm_send(const u32 chip_num, unsigned char *cmd,
> - size_t buflen)
> +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> {
> int rc;
>
> dump_tpm_buf(cmd);
> - rc = tpm_send(chip_num, cmd, buflen);
> + rc = tpm_send(NULL, cmd, buflen);
> dump_tpm_buf(cmd);
> if (rc > 0)
> /* Can't return positive return codes values to keyctl */
> @@ -382,10 +381,10 @@ static int pcrlock(const int pcrnum)
>
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> + ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
> if (ret != SHA1_DIGEST_SIZE)
> return ret;
> - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> + return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
> }
>
> /*
> @@ -398,7 +397,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> unsigned char ononce[TPM_NONCE_SIZE];
> int ret;
>
> - ret = tpm_get_random(TPM_ANY_NUM, ononce, TPM_NONCE_SIZE);
> + ret = tpm_get_random(NULL, ononce, TPM_NONCE_SIZE);
> if (ret != TPM_NONCE_SIZE)
> return ret;
>
> @@ -410,7 +409,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> store32(tb, handle);
> storebytes(tb, ononce, TPM_NONCE_SIZE);
>
> - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> if (ret < 0)
> return ret;
>
> @@ -434,7 +433,7 @@ static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> store16(tb, TPM_TAG_RQU_COMMAND);
> store32(tb, TPM_OIAP_SIZE);
> store32(tb, TPM_ORD_OIAP);
> - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> if (ret < 0)
> return ret;
>
> @@ -493,7 +492,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> if (ret < 0)
> goto out;
>
> - ret = tpm_get_random(TPM_ANY_NUM, td->nonceodd, TPM_NONCE_SIZE);
> + ret = tpm_get_random(NULL, td->nonceodd, TPM_NONCE_SIZE);
> if (ret != TPM_NONCE_SIZE)
> goto out;
> ordinal = htonl(TPM_ORD_SEAL);
> @@ -542,7 +541,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> store8(tb, cont);
> storebytes(tb, td->pubauth, SHA1_DIGEST_SIZE);
>
> - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> if (ret < 0)
> goto out;
>
> @@ -603,7 +602,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>
> ordinal = htonl(TPM_ORD_UNSEAL);
> keyhndl = htonl(SRKHANDLE);
> - ret = tpm_get_random(TPM_ANY_NUM, nonceodd, TPM_NONCE_SIZE);
> + ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE);
> if (ret != TPM_NONCE_SIZE) {
> pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
> return ret;
> @@ -635,7 +634,7 @@ static int tpm_unseal(struct tpm_buf *tb,
> store8(tb, cont);
> storebytes(tb, authdata2, SHA1_DIGEST_SIZE);
>
> - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> if (ret < 0) {
> pr_info("trusted_key: authhmac failed (%d)\n", ret);
> return ret;
> @@ -748,7 +747,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> int i;
> int tpm2;
>
> - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> + tpm2 = tpm_is_tpm2(NULL);
> if (tpm2 < 0)
> return tpm2;
>
> @@ -917,7 +916,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
> struct trusted_key_options *options;
> int tpm2;
>
> - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> + tpm2 = tpm_is_tpm2(NULL);
> if (tpm2 < 0)
> return NULL;
>
> @@ -967,7 +966,7 @@ static int trusted_instantiate(struct key *key,
> size_t key_len;
> int tpm2;
>
> - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> + tpm2 = tpm_is_tpm2(NULL);
> if (tpm2 < 0)
> return tpm2;
>
> @@ -1008,7 +1007,7 @@ static int trusted_instantiate(struct key *key,
> switch (key_cmd) {
> case Opt_load:
> if (tpm2)
> - ret = tpm_unseal_trusted(TPM_ANY_NUM, payload, options);
> + ret = tpm_unseal_trusted(NULL, payload, options);
> else
> ret = key_unseal(payload, options);
> dump_payload(payload);
> @@ -1018,13 +1017,13 @@ static int trusted_instantiate(struct key *key,
> break;
> case Opt_new:
> key_len = payload->key_len;
> - ret = tpm_get_random(TPM_ANY_NUM, payload->key, key_len);
> + ret = tpm_get_random(NULL, payload->key, key_len);
> if (ret != key_len) {
> pr_info("trusted_key: key_create failed (%d)\n", ret);
> goto out;
> }
> if (tpm2)
> - ret = tpm_seal_trusted(TPM_ANY_NUM, payload, options);
> + ret = tpm_seal_trusted(NULL, payload, options);
> else
> ret = key_seal(payload, options);
> if (ret < 0)
> --
> 2.14.1
>

Looks good to me. FWIW, Reviewed-by: PrasannaKumar Muralidharan
<[email protected]>.

Regards,
PrasannaKumar

2017-10-26 16:09:12

by Jarkko Sakkinen

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Thu, Oct 26, 2017 at 07:40:49PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Jarkko,
>
> On 26 October 2017 at 19:24, Jarkko Sakkinen
> <[email protected]> wrote:
> > Device number (the character device index) is not a stable identifier
> > for a TPM chip. That is the reason why every call site passes
> > TPM_ANY_NUM to tpm_chip_find_get().
> >
> > This commit changes the API in a way that instead a struct tpm_chip
> > instance is given and NULL means the default chip. In addition, this
> > commit refines the documentation to be up to date with the
> > implementation.
> >
> > Suggested-by: Jason Gunthorpe <[email protected]> (@chip_num -> @chip)
> > Signed-off-by: Jarkko Sakkinen <[email protected]>
> > ---
> > v3:
> > * If chip != NULL was given to tpm_chip_find_get(), it tried to reserve chip
> > associated with the device number 0 instead of trying to reserve the given
> > chip instance.
> > v2:
> > * Further defined function documentation.
> > * Changed @chip_num to @chip instead of removing the parameter as suggested by
> > Jason Gunthorpe.
> > drivers/char/hw_random/tpm-rng.c | 2 +-
> > drivers/char/tpm/tpm-chip.c | 24 ++++---
> > drivers/char/tpm/tpm-interface.c | 135 +++++++++++++++++++-----------------
> > drivers/char/tpm/tpm.h | 2 +-
> > include/linux/tpm.h | 38 +++++-----
> > security/integrity/ima/ima_crypto.c | 2 +-
> > security/integrity/ima/ima_init.c | 2 +-
> > security/integrity/ima/ima_queue.c | 2 +-
> > security/keys/trusted.c | 35 +++++-----
> > 9 files changed, 126 insertions(+), 116 deletions(-)
> >
> > diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
> > index d6d448266f07..c5e363825af0 100644
> > --- a/drivers/char/hw_random/tpm-rng.c
> > +++ b/drivers/char/hw_random/tpm-rng.c
> > @@ -25,7 +25,7 @@
> >
> > static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> > {
> > - return tpm_get_random(TPM_ANY_NUM, data, max);
> > + return tpm_get_random(NULL, data, max);
> > }
> >
> > static struct hwrng tpm_rng = {
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index a114e8f7fb90..bab9c14e040c 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
> > EXPORT_SYMBOL_GPL(tpm_put_ops);
> >
> > /**
> > - * tpm_chip_find_get() - return tpm_chip for a given chip number
> > - * @chip_num: id to find
> > + * tpm_chip_find_get() - find and reserve a TPM chip
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > *
> > - * The return'd chip has been tpm_try_get_ops'd and must be released via
> > - * tpm_put_ops
> > + * Finds a TPM chip and reserves its class device and operations. The chip must
> > + * be released with tpm_chip_put_ops() after use.
> > + *
> > + * Return:
> > + * A reserved &struct tpm_chip instance.
> > + * %NULL if a chip is not found.
> > + * %NULL if the chip is not available.
> > */
> > -struct tpm_chip *tpm_chip_find_get(int chip_num)
> > +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
> > {
> > - struct tpm_chip *chip, *res = NULL;
> > + struct tpm_chip *res = NULL;
> > + int chip_num = 0;
> > int chip_prev;
> >
> > mutex_lock(&idr_lock);
> >
> > - if (chip_num == TPM_ANY_NUM) {
> > - chip_num = 0;
> > + if (!chip) {
> > do {
> > chip_prev = chip_num;
> > chip = idr_get_next(&dev_nums_idr, &chip_num);
> > @@ -105,8 +110,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
> > }
> > } while (chip_prev != chip_num);
> > } else {
> > - chip = idr_find(&dev_nums_idr, chip_num);
> > - if (chip && !tpm_try_get_ops(chip))
> > + if (!tpm_try_get_ops(chip))
> > res = chip;
> > }
> >
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index ebe0a1d36d8c..19f820f775b5 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> > }
> >
> > /**
> > - * tpm_is_tpm2 - is the chip a TPM2 chip?
> > - * @chip_num: tpm idx # or ANY
> > + * tpm_is_tpm2 - do we a have a TPM2 chip?
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > *
> > - * Returns < 0 on error, and 1 or 0 on success depending whether the chip
> > - * is a TPM2 chip.
> > + * Return:
> > + * 1 if we have a TPM2 chip.
> > + * 0 if we don't have a TPM2 chip.
> > + * A negative number for system errors (errno).
> > */
> > -int tpm_is_tpm2(u32 chip_num)
> > +int tpm_is_tpm2(struct tpm_chip *chip)
> > {
> > - struct tpm_chip *chip;
> > int rc;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL)
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip)
> > return -ENODEV;
> >
> > rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
> > @@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
> > EXPORT_SYMBOL_GPL(tpm_is_tpm2);
> >
> > /**
> > - * tpm_pcr_read - read a pcr value
> > - * @chip_num: tpm idx # or ANY
> > - * @pcr_idx: pcr idx to retrieve
> > - * @res_buf: TPM_PCR value
> > - * size of res_buf is 20 bytes (or NULL if you don't care)
> > + * tpm_pcr_read - read a PCR value from SHA1 bank
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @pcr_idx: the PCR to be retrieved
> > + * @res_buf: the value of the PCR
> > *
> > - * The TPM driver should be built-in, but for whatever reason it
> > - * isn't, protect against the chip disappearing, by incrementing
> > - * the module usage count.
> > + * Return: same as with tpm_transmit_cmd()
> > */
> > -int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
> > +int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> > {
> > - struct tpm_chip *chip;
> > int rc;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL)
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip)
> > return -ENODEV;
> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
> > @@ -889,25 +886,26 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
> > }
> >
> > /**
> > - * tpm_pcr_extend - extend pcr value with hash
> > - * @chip_num: tpm idx # or AN&
> > - * @pcr_idx: pcr idx to extend
> > - * @hash: hash value used to extend pcr value
> > + * tpm_pcr_extend - extend a PCR value in SHA1 bank.
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @pcr_idx: the PCR to be retrieved
> > + * @hash: the hash value used to extend the PCR value
> > *
> > - * The TPM driver should be built-in, but for whatever reason it
> > - * isn't, protect against the chip disappearing, by incrementing
> > - * the module usage count.
> > + * Note: with TPM 2.0 extends also those banks with a known digest size to the
> > + * cryto subsystem in order to prevent malicious use of those PCR banks. In the
> > + * future we should dynamically determine digest sizes.
> > + *
> > + * Return: same as with tpm_transmit_cmd()
> > */
> > -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> > +int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
> > {
> > int rc;
> > - struct tpm_chip *chip;
> > struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> > u32 count = 0;
> > int i;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL)
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip)
> > return -ENODEV;
> >
> > if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > @@ -1019,17 +1017,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
> > return rc;
> > }
> >
> > -int tpm_send(u32 chip_num, void *cmd, size_t buflen)
> > +/**
> > + * tpm_send - send a TPM command
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @cmd: a TPM command buffer
> > + * @buflen: the length of the TPM command buffer
> > + *
> > + * Return: same as with tpm_transmit_cmd()
> > + */
> > +int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> > {
> > - struct tpm_chip *chip;
> > int rc;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL)
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip)
> > return -ENODEV;
> >
> > rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
> > - "attempting tpm_cmd");
> > + "attempting to a send a command");
> > tpm_put_ops(chip);
> > return rc;
> > }
> > @@ -1127,16 +1132,15 @@ static const struct tpm_input_header tpm_getrandom_header = {
> > };
> >
> > /**
> > - * tpm_get_random() - Get random bytes from the tpm's RNG
> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> > - * @out: destination buffer for the random bytes
> > - * @max: the max number of bytes to write to @out
> > + * tpm_get_random() - get random bytes from the TPM's RNG
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @out: destination buffer for the random bytes
> > + * @max: the max number of bytes to write to @out
> > *
> > - * Returns < 0 on error and the number of bytes read on success
> > + * Return: same as with tpm_transmit_cmd()
> > */
> > -int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> > +int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> > {
> > - struct tpm_chip *chip;
> > struct tpm_cmd_t tpm_cmd;
> > u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
> > int err, total = 0, retries = 5;
> > @@ -1145,8 +1149,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> > if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
> > return -EINVAL;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL)
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip)
> > return -ENODEV;
> >
> > if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > @@ -1188,22 +1192,23 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> > EXPORT_SYMBOL_GPL(tpm_get_random);
> >
> > /**
> > - * tpm_seal_trusted() - seal a trusted key
> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> > - * @options: authentication values and other options
> > - * @payload: the key data in clear and encrypted form
> > + * tpm_seal_trusted() - seal a trusted key payload
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @options: authentication values and other options
> > + * @payload: the key data in clear and encrypted form
> > + *
> > + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
> > + * the keyring subsystem.
> > *
> > - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
> > - * are supported.
> > + * Return: same as with tpm_transmit_cmd()
> > */
> > -int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
> > +int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
> > struct trusted_key_options *options)
> > {
> > - struct tpm_chip *chip;
> > int rc;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> > return -ENODEV;
> >
> > rc = tpm2_seal_trusted(chip, payload, options);
> > @@ -1215,21 +1220,23 @@ EXPORT_SYMBOL_GPL(tpm_seal_trusted);
> >
> > /**
> > * tpm_unseal_trusted() - unseal a trusted key
> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
> > - * @options: authentication values and other options
> > - * @payload: the key data in clear and encrypted form
> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
> > + * @options: authentication values and other options
> > + * @payload: the key data in clear and encrypted form
> > + *
> > + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
> > + * the keyring subsystem.
> > *
> > - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
> > - * are supported.
> > + * Return: same as with tpm_transmit_cmd()
> > */
> > -int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
> > +int tpm_unseal_trusted(struct tpm_chip *chip,
> > + struct trusted_key_payload *payload,
> > struct trusted_key_options *options)
> > {
> > - struct tpm_chip *chip;
> > int rc;
> >
> > - chip = tpm_chip_find_get(chip_num);
> > - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> > + chip = tpm_chip_find_get(chip);
> > + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
> > return -ENODEV;
> >
> > rc = tpm2_unseal_trusted(chip, payload, options);
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index c1866cc02e30..6c189174c0d3 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -516,7 +516,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
> > delay_msec * 1000);
> > };
> >
> > -struct tpm_chip *tpm_chip_find_get(int chip_num);
> > +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip);
> > __must_check int tpm_try_get_ops(struct tpm_chip *chip);
> > void tpm_put_ops(struct tpm_chip *chip);
> >
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index 5a090f5ab335..ddc9b88ff6d3 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -24,11 +24,6 @@
> >
> > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> >
> > -/*
> > - * Chip num is this value or a valid tpm idx
> > - */
> > -#define TPM_ANY_NUM 0xFFFF
> > -
> > struct tpm_chip;
> > struct trusted_key_payload;
> > struct trusted_key_options;
> > @@ -54,42 +49,47 @@ struct tpm_class_ops {
> >
> > #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
> >
> > -extern int tpm_is_tpm2(u32 chip_num);
> > -extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
> > -extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> > -extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
> > -extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
> > -extern int tpm_seal_trusted(u32 chip_num,
> > +extern int tpm_is_tpm2(struct tpm_chip *chip);
> > +extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> > +extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> > +extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
> > +extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
> > +extern int tpm_seal_trusted(struct tpm_chip *chip,
> > struct trusted_key_payload *payload,
> > struct trusted_key_options *options);
> > -extern int tpm_unseal_trusted(u32 chip_num,
> > +extern int tpm_unseal_trusted(struct tpm_chip *chip,
> > struct trusted_key_payload *payload,
> > struct trusted_key_options *options);
> > #else
> > -static inline int tpm_is_tpm2(u32 chip_num)
> > +static inline int tpm_is_tpm2(struct tpm_chip *chip)
> > {
> > return -ENODEV;
> > }
> > -static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
> > +static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> > +{
> > return -ENODEV;
> > }
> > -static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
> > +static inline int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx,
> > + const u8 *hash)
> > +{
> > return -ENODEV;
> > }
> > -static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
> > +static inline int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> > +{
> > return -ENODEV;
> > }
> > -static inline int tpm_get_random(u32 chip_num, u8 *data, size_t max) {
> > +static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
> > +{
> > return -ENODEV;
> > }
> >
> > -static inline int tpm_seal_trusted(u32 chip_num,
> > +static inline int tpm_seal_trusted(struct tpm_chip *chip,
> > struct trusted_key_payload *payload,
> > struct trusted_key_options *options)
> > {
> > return -ENODEV;
> > }
> > -static inline int tpm_unseal_trusted(u32 chip_num,
> > +static inline int tpm_unseal_trusted(struct tpm_chip *chip,
> > struct trusted_key_payload *payload,
> > struct trusted_key_options *options)
> > {
> > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> > index 802d5d20f36f..3371d134ee62 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -644,7 +644,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
> > if (!ima_used_chip)
> > return;
> >
> > - if (tpm_pcr_read(TPM_ANY_NUM, idx, pcr) != 0)
> > + if (tpm_pcr_read(NULL, idx, pcr) != 0)
> > pr_err("Error Communicating to TPM chip\n");
> > }
> >
> > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > index 2967d497a665..29b72cd2502e 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -110,7 +110,7 @@ int __init ima_init(void)
> > int rc;
> >
> > ima_used_chip = 0;
> > - rc = tpm_pcr_read(TPM_ANY_NUM, 0, pcr_i);
> > + rc = tpm_pcr_read(NULL, 0, pcr_i);
> > if (rc == 0)
> > ima_used_chip = 1;
> >
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index a02a86d51102..418f35e38015 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -145,7 +145,7 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
> > if (!ima_used_chip)
> > return result;
> >
> > - result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
> > + result = tpm_pcr_extend(NULL, pcr, hash);
> > if (result != 0)
> > pr_err("Error Communicating to TPM chip, result: %d\n", result);
> > return result;
> > diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> > index ddfaebf60fc8..d2b20b1cc2f1 100644
> > --- a/security/keys/trusted.c
> > +++ b/security/keys/trusted.c
> > @@ -355,13 +355,12 @@ static int TSS_checkhmac2(unsigned char *buffer,
> > * For key specific tpm requests, we will generate and send our
> > * own TPM command packets using the drivers send function.
> > */
> > -static int trusted_tpm_send(const u32 chip_num, unsigned char *cmd,
> > - size_t buflen)
> > +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > {
> > int rc;
> >
> > dump_tpm_buf(cmd);
> > - rc = tpm_send(chip_num, cmd, buflen);
> > + rc = tpm_send(NULL, cmd, buflen);
> > dump_tpm_buf(cmd);
> > if (rc > 0)
> > /* Can't return positive return codes values to keyctl */
> > @@ -382,10 +381,10 @@ static int pcrlock(const int pcrnum)
> >
> > if (!capable(CAP_SYS_ADMIN))
> > return -EPERM;
> > - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> > + ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
> > if (ret != SHA1_DIGEST_SIZE)
> > return ret;
> > - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> > + return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
> > }
> >
> > /*
> > @@ -398,7 +397,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> > unsigned char ononce[TPM_NONCE_SIZE];
> > int ret;
> >
> > - ret = tpm_get_random(TPM_ANY_NUM, ononce, TPM_NONCE_SIZE);
> > + ret = tpm_get_random(NULL, ononce, TPM_NONCE_SIZE);
> > if (ret != TPM_NONCE_SIZE)
> > return ret;
> >
> > @@ -410,7 +409,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> > store32(tb, handle);
> > storebytes(tb, ononce, TPM_NONCE_SIZE);
> >
> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> > if (ret < 0)
> > return ret;
> >
> > @@ -434,7 +433,7 @@ static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> > store16(tb, TPM_TAG_RQU_COMMAND);
> > store32(tb, TPM_OIAP_SIZE);
> > store32(tb, TPM_ORD_OIAP);
> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> > if (ret < 0)
> > return ret;
> >
> > @@ -493,7 +492,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> > if (ret < 0)
> > goto out;
> >
> > - ret = tpm_get_random(TPM_ANY_NUM, td->nonceodd, TPM_NONCE_SIZE);
> > + ret = tpm_get_random(NULL, td->nonceodd, TPM_NONCE_SIZE);
> > if (ret != TPM_NONCE_SIZE)
> > goto out;
> > ordinal = htonl(TPM_ORD_SEAL);
> > @@ -542,7 +541,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> > store8(tb, cont);
> > storebytes(tb, td->pubauth, SHA1_DIGEST_SIZE);
> >
> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> > if (ret < 0)
> > goto out;
> >
> > @@ -603,7 +602,7 @@ static int tpm_unseal(struct tpm_buf *tb,
> >
> > ordinal = htonl(TPM_ORD_UNSEAL);
> > keyhndl = htonl(SRKHANDLE);
> > - ret = tpm_get_random(TPM_ANY_NUM, nonceodd, TPM_NONCE_SIZE);
> > + ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE);
> > if (ret != TPM_NONCE_SIZE) {
> > pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
> > return ret;
> > @@ -635,7 +634,7 @@ static int tpm_unseal(struct tpm_buf *tb,
> > store8(tb, cont);
> > storebytes(tb, authdata2, SHA1_DIGEST_SIZE);
> >
> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
> > if (ret < 0) {
> > pr_info("trusted_key: authhmac failed (%d)\n", ret);
> > return ret;
> > @@ -748,7 +747,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > int i;
> > int tpm2;
> >
> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> > + tpm2 = tpm_is_tpm2(NULL);
> > if (tpm2 < 0)
> > return tpm2;
> >
> > @@ -917,7 +916,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
> > struct trusted_key_options *options;
> > int tpm2;
> >
> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> > + tpm2 = tpm_is_tpm2(NULL);
> > if (tpm2 < 0)
> > return NULL;
> >
> > @@ -967,7 +966,7 @@ static int trusted_instantiate(struct key *key,
> > size_t key_len;
> > int tpm2;
> >
> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
> > + tpm2 = tpm_is_tpm2(NULL);
> > if (tpm2 < 0)
> > return tpm2;
> >
> > @@ -1008,7 +1007,7 @@ static int trusted_instantiate(struct key *key,
> > switch (key_cmd) {
> > case Opt_load:
> > if (tpm2)
> > - ret = tpm_unseal_trusted(TPM_ANY_NUM, payload, options);
> > + ret = tpm_unseal_trusted(NULL, payload, options);
> > else
> > ret = key_unseal(payload, options);
> > dump_payload(payload);
> > @@ -1018,13 +1017,13 @@ static int trusted_instantiate(struct key *key,
> > break;
> > case Opt_new:
> > key_len = payload->key_len;
> > - ret = tpm_get_random(TPM_ANY_NUM, payload->key, key_len);
> > + ret = tpm_get_random(NULL, payload->key, key_len);
> > if (ret != key_len) {
> > pr_info("trusted_key: key_create failed (%d)\n", ret);
> > goto out;
> > }
> > if (tpm2)
> > - ret = tpm_seal_trusted(TPM_ANY_NUM, payload, options);
> > + ret = tpm_seal_trusted(NULL, payload, options);
> > else
> > ret = key_seal(payload, options);
> > if (ret < 0)
> > --
> > 2.14.1
> >
>
> Looks good to me. FWIW, Reviewed-by: PrasannaKumar Muralidharan
> <[email protected]>.
>
> Regards,
> PrasannaKumar

Thank you.

/Jarkko

2017-10-26 16:27:12

by PrasannaKumar Muralidharan

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On 26 October 2017 at 21:39, Jarkko Sakkinen
<jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]> wrote:
> On Thu, Oct 26, 2017 at 07:40:49PM +0530, PrasannaKumar Muralidharan wrote:
>> Hi Jarkko,
>>
>> On 26 October 2017 at 19:24, Jarkko Sakkinen
>> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]> wrote:
>> > Device number (the character device index) is not a stable identifier
>> > for a TPM chip. That is the reason why every call site passes
>> > TPM_ANY_NUM to tpm_chip_find_get().
>> >
>> > This commit changes the API in a way that instead a struct tpm_chip
>> > instance is given and NULL means the default chip. In addition, this
>> > commit refines the documentation to be up to date with the
>> > implementation.
>> >
>> > Suggested-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> (@chip_num -> @chip)
>> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/[email protected]>
>> > ---
>> > v3:
>> > * If chip != NULL was given to tpm_chip_find_get(), it tried to reserve chip
>> > associated with the device number 0 instead of trying to reserve the given
>> > chip instance.
>> > v2:
>> > * Further defined function documentation.
>> > * Changed @chip_num to @chip instead of removing the parameter as suggested by
>> > Jason Gunthorpe.
>> > drivers/char/hw_random/tpm-rng.c | 2 +-
>> > drivers/char/tpm/tpm-chip.c | 24 ++++---
>> > drivers/char/tpm/tpm-interface.c | 135 +++++++++++++++++++-----------------
>> > drivers/char/tpm/tpm.h | 2 +-
>> > include/linux/tpm.h | 38 +++++-----
>> > security/integrity/ima/ima_crypto.c | 2 +-
>> > security/integrity/ima/ima_init.c | 2 +-
>> > security/integrity/ima/ima_queue.c | 2 +-
>> > security/keys/trusted.c | 35 +++++-----
>> > 9 files changed, 126 insertions(+), 116 deletions(-)
>> >
>> > diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
>> > index d6d448266f07..c5e363825af0 100644
>> > --- a/drivers/char/hw_random/tpm-rng.c
>> > +++ b/drivers/char/hw_random/tpm-rng.c
>> > @@ -25,7 +25,7 @@
>> >
>> > static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>> > {
>> > - return tpm_get_random(TPM_ANY_NUM, data, max);
>> > + return tpm_get_random(NULL, data, max);
>> > }
>> >
>> > static struct hwrng tpm_rng = {
>> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> > index a114e8f7fb90..bab9c14e040c 100644
>> > --- a/drivers/char/tpm/tpm-chip.c
>> > +++ b/drivers/char/tpm/tpm-chip.c
>> > @@ -81,21 +81,26 @@ void tpm_put_ops(struct tpm_chip *chip)
>> > EXPORT_SYMBOL_GPL(tpm_put_ops);
>> >
>> > /**
>> > - * tpm_chip_find_get() - return tpm_chip for a given chip number
>> > - * @chip_num: id to find
>> > + * tpm_chip_find_get() - find and reserve a TPM chip
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > *
>> > - * The return'd chip has been tpm_try_get_ops'd and must be released via
>> > - * tpm_put_ops
>> > + * Finds a TPM chip and reserves its class device and operations. The chip must
>> > + * be released with tpm_chip_put_ops() after use.
>> > + *
>> > + * Return:
>> > + * A reserved &struct tpm_chip instance.
>> > + * %NULL if a chip is not found.
>> > + * %NULL if the chip is not available.
>> > */
>> > -struct tpm_chip *tpm_chip_find_get(int chip_num)
>> > +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
>> > {
>> > - struct tpm_chip *chip, *res = NULL;
>> > + struct tpm_chip *res = NULL;
>> > + int chip_num = 0;
>> > int chip_prev;
>> >
>> > mutex_lock(&idr_lock);
>> >
>> > - if (chip_num == TPM_ANY_NUM) {
>> > - chip_num = 0;
>> > + if (!chip) {
>> > do {
>> > chip_prev = chip_num;
>> > chip = idr_get_next(&dev_nums_idr, &chip_num);
>> > @@ -105,8 +110,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
>> > }
>> > } while (chip_prev != chip_num);
>> > } else {
>> > - chip = idr_find(&dev_nums_idr, chip_num);
>> > - if (chip && !tpm_try_get_ops(chip))
>> > + if (!tpm_try_get_ops(chip))
>> > res = chip;
>> > }
>> >
>> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> > index ebe0a1d36d8c..19f820f775b5 100644
>> > --- a/drivers/char/tpm/tpm-interface.c
>> > +++ b/drivers/char/tpm/tpm-interface.c
>> > @@ -809,19 +809,20 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>> > }
>> >
>> > /**
>> > - * tpm_is_tpm2 - is the chip a TPM2 chip?
>> > - * @chip_num: tpm idx # or ANY
>> > + * tpm_is_tpm2 - do we a have a TPM2 chip?
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > *
>> > - * Returns < 0 on error, and 1 or 0 on success depending whether the chip
>> > - * is a TPM2 chip.
>> > + * Return:
>> > + * 1 if we have a TPM2 chip.
>> > + * 0 if we don't have a TPM2 chip.
>> > + * A negative number for system errors (errno).
>> > */
>> > -int tpm_is_tpm2(u32 chip_num)
>> > +int tpm_is_tpm2(struct tpm_chip *chip)
>> > {
>> > - struct tpm_chip *chip;
>> > int rc;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL)
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip)
>> > return -ENODEV;
>> >
>> > rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
>> > @@ -833,23 +834,19 @@ int tpm_is_tpm2(u32 chip_num)
>> > EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>> >
>> > /**
>> > - * tpm_pcr_read - read a pcr value
>> > - * @chip_num: tpm idx # or ANY
>> > - * @pcr_idx: pcr idx to retrieve
>> > - * @res_buf: TPM_PCR value
>> > - * size of res_buf is 20 bytes (or NULL if you don't care)
>> > + * tpm_pcr_read - read a PCR value from SHA1 bank
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @pcr_idx: the PCR to be retrieved
>> > + * @res_buf: the value of the PCR
>> > *
>> > - * The TPM driver should be built-in, but for whatever reason it
>> > - * isn't, protect against the chip disappearing, by incrementing
>> > - * the module usage count.
>> > + * Return: same as with tpm_transmit_cmd()
>> > */
>> > -int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
>> > +int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>> > {
>> > - struct tpm_chip *chip;
>> > int rc;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL)
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip)
>> > return -ENODEV;
>> > if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> > rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
>> > @@ -889,25 +886,26 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
>> > }
>> >
>> > /**
>> > - * tpm_pcr_extend - extend pcr value with hash
>> > - * @chip_num: tpm idx # or AN&
>> > - * @pcr_idx: pcr idx to extend
>> > - * @hash: hash value used to extend pcr value
>> > + * tpm_pcr_extend - extend a PCR value in SHA1 bank.
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @pcr_idx: the PCR to be retrieved
>> > + * @hash: the hash value used to extend the PCR value
>> > *
>> > - * The TPM driver should be built-in, but for whatever reason it
>> > - * isn't, protect against the chip disappearing, by incrementing
>> > - * the module usage count.
>> > + * Note: with TPM 2.0 extends also those banks with a known digest size to the
>> > + * cryto subsystem in order to prevent malicious use of those PCR banks. In the
>> > + * future we should dynamically determine digest sizes.
>> > + *
>> > + * Return: same as with tpm_transmit_cmd()
>> > */
>> > -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>> > +int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
>> > {
>> > int rc;
>> > - struct tpm_chip *chip;
>> > struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
>> > u32 count = 0;
>> > int i;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL)
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip)
>> > return -ENODEV;
>> >
>> > if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> > @@ -1019,17 +1017,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>> > return rc;
>> > }
>> >
>> > -int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>> > +/**
>> > + * tpm_send - send a TPM command
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @cmd: a TPM command buffer
>> > + * @buflen: the length of the TPM command buffer
>> > + *
>> > + * Return: same as with tpm_transmit_cmd()
>> > + */
>> > +int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
>> > {
>> > - struct tpm_chip *chip;
>> > int rc;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL)
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip)
>> > return -ENODEV;
>> >
>> > rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
>> > - "attempting tpm_cmd");
>> > + "attempting to a send a command");
>> > tpm_put_ops(chip);
>> > return rc;
>> > }
>> > @@ -1127,16 +1132,15 @@ static const struct tpm_input_header tpm_getrandom_header = {
>> > };
>> >
>> > /**
>> > - * tpm_get_random() - Get random bytes from the tpm's RNG
>> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
>> > - * @out: destination buffer for the random bytes
>> > - * @max: the max number of bytes to write to @out
>> > + * tpm_get_random() - get random bytes from the TPM's RNG
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @out: destination buffer for the random bytes
>> > + * @max: the max number of bytes to write to @out
>> > *
>> > - * Returns < 0 on error and the number of bytes read on success
>> > + * Return: same as with tpm_transmit_cmd()
>> > */
>> > -int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>> > +int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>> > {
>> > - struct tpm_chip *chip;
>> > struct tpm_cmd_t tpm_cmd;
>> > u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
>> > int err, total = 0, retries = 5;
>> > @@ -1145,8 +1149,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>> > if (!out || !num_bytes || max > TPM_MAX_RNG_DATA)
>> > return -EINVAL;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL)
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip)
>> > return -ENODEV;
>> >
>> > if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> > @@ -1188,22 +1192,23 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>> > EXPORT_SYMBOL_GPL(tpm_get_random);
>> >
>> > /**
>> > - * tpm_seal_trusted() - seal a trusted key
>> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
>> > - * @options: authentication values and other options
>> > - * @payload: the key data in clear and encrypted form
>> > + * tpm_seal_trusted() - seal a trusted key payload
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @options: authentication values and other options
>> > + * @payload: the key data in clear and encrypted form
>> > + *
>> > + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
>> > + * the keyring subsystem.
>> > *
>> > - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
>> > - * are supported.
>> > + * Return: same as with tpm_transmit_cmd()
>> > */
>> > -int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
>> > +int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
>> > struct trusted_key_options *options)
>> > {
>> > - struct tpm_chip *chip;
>> > int rc;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
>> > return -ENODEV;
>> >
>> > rc = tpm2_seal_trusted(chip, payload, options);
>> > @@ -1215,21 +1220,23 @@ EXPORT_SYMBOL_GPL(tpm_seal_trusted);
>> >
>> > /**
>> > * tpm_unseal_trusted() - unseal a trusted key
>> > - * @chip_num: A specific chip number for the request or TPM_ANY_NUM
>> > - * @options: authentication values and other options
>> > - * @payload: the key data in clear and encrypted form
>> > + * @chip: a &struct tpm_chip instance, %NULL for the default chip
>> > + * @options: authentication values and other options
>> > + * @payload: the key data in clear and encrypted form
>> > + *
>> > + * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
>> > + * the keyring subsystem.
>> > *
>> > - * Returns < 0 on error and 0 on success. At the moment, only TPM 2.0 chips
>> > - * are supported.
>> > + * Return: same as with tpm_transmit_cmd()
>> > */
>> > -int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
>> > +int tpm_unseal_trusted(struct tpm_chip *chip,
>> > + struct trusted_key_payload *payload,
>> > struct trusted_key_options *options)
>> > {
>> > - struct tpm_chip *chip;
>> > int rc;
>> >
>> > - chip = tpm_chip_find_get(chip_num);
>> > - if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
>> > + chip = tpm_chip_find_get(chip);
>> > + if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
>> > return -ENODEV;
>> >
>> > rc = tpm2_unseal_trusted(chip, payload, options);
>> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> > index c1866cc02e30..6c189174c0d3 100644
>> > --- a/drivers/char/tpm/tpm.h
>> > +++ b/drivers/char/tpm/tpm.h
>> > @@ -516,7 +516,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
>> > delay_msec * 1000);
>> > };
>> >
>> > -struct tpm_chip *tpm_chip_find_get(int chip_num);
>> > +struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip);
>> > __must_check int tpm_try_get_ops(struct tpm_chip *chip);
>> > void tpm_put_ops(struct tpm_chip *chip);
>> >
>> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> > index 5a090f5ab335..ddc9b88ff6d3 100644
>> > --- a/include/linux/tpm.h
>> > +++ b/include/linux/tpm.h
>> > @@ -24,11 +24,6 @@
>> >
>> > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>> >
>> > -/*
>> > - * Chip num is this value or a valid tpm idx
>> > - */
>> > -#define TPM_ANY_NUM 0xFFFF
>> > -
>> > struct tpm_chip;
>> > struct trusted_key_payload;
>> > struct trusted_key_options;
>> > @@ -54,42 +49,47 @@ struct tpm_class_ops {
>> >
>> > #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>> >
>> > -extern int tpm_is_tpm2(u32 chip_num);
>> > -extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
>> > -extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
>> > -extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
>> > -extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
>> > -extern int tpm_seal_trusted(u32 chip_num,
>> > +extern int tpm_is_tpm2(struct tpm_chip *chip);
>> > +extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>> > +extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
>> > +extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
>> > +extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
>> > +extern int tpm_seal_trusted(struct tpm_chip *chip,
>> > struct trusted_key_payload *payload,
>> > struct trusted_key_options *options);
>> > -extern int tpm_unseal_trusted(u32 chip_num,
>> > +extern int tpm_unseal_trusted(struct tpm_chip *chip,
>> > struct trusted_key_payload *payload,
>> > struct trusted_key_options *options);
>> > #else
>> > -static inline int tpm_is_tpm2(u32 chip_num)
>> > +static inline int tpm_is_tpm2(struct tpm_chip *chip)
>> > {
>> > return -ENODEV;
>> > }
>> > -static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
>> > +static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>> > +{
>> > return -ENODEV;
>> > }
>> > -static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
>> > +static inline int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx,
>> > + const u8 *hash)
>> > +{
>> > return -ENODEV;
>> > }
>> > -static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
>> > +static inline int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
>> > +{
>> > return -ENODEV;
>> > }
>> > -static inline int tpm_get_random(u32 chip_num, u8 *data, size_t max) {
>> > +static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
>> > +{
>> > return -ENODEV;
>> > }
>> >
>> > -static inline int tpm_seal_trusted(u32 chip_num,
>> > +static inline int tpm_seal_trusted(struct tpm_chip *chip,
>> > struct trusted_key_payload *payload,
>> > struct trusted_key_options *options)
>> > {
>> > return -ENODEV;
>> > }
>> > -static inline int tpm_unseal_trusted(u32 chip_num,
>> > +static inline int tpm_unseal_trusted(struct tpm_chip *chip,
>> > struct trusted_key_payload *payload,
>> > struct trusted_key_options *options)
>> > {
>> > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
>> > index 802d5d20f36f..3371d134ee62 100644
>> > --- a/security/integrity/ima/ima_crypto.c
>> > +++ b/security/integrity/ima/ima_crypto.c
>> > @@ -644,7 +644,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
>> > if (!ima_used_chip)
>> > return;
>> >
>> > - if (tpm_pcr_read(TPM_ANY_NUM, idx, pcr) != 0)
>> > + if (tpm_pcr_read(NULL, idx, pcr) != 0)
>> > pr_err("Error Communicating to TPM chip\n");
>> > }
>> >
>> > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
>> > index 2967d497a665..29b72cd2502e 100644
>> > --- a/security/integrity/ima/ima_init.c
>> > +++ b/security/integrity/ima/ima_init.c
>> > @@ -110,7 +110,7 @@ int __init ima_init(void)
>> > int rc;
>> >
>> > ima_used_chip = 0;
>> > - rc = tpm_pcr_read(TPM_ANY_NUM, 0, pcr_i);
>> > + rc = tpm_pcr_read(NULL, 0, pcr_i);
>> > if (rc == 0)
>> > ima_used_chip = 1;
>> >
>> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> > index a02a86d51102..418f35e38015 100644
>> > --- a/security/integrity/ima/ima_queue.c
>> > +++ b/security/integrity/ima/ima_queue.c
>> > @@ -145,7 +145,7 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
>> > if (!ima_used_chip)
>> > return result;
>> >
>> > - result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
>> > + result = tpm_pcr_extend(NULL, pcr, hash);
>> > if (result != 0)
>> > pr_err("Error Communicating to TPM chip, result: %d\n", result);
>> > return result;
>> > diff --git a/security/keys/trusted.c b/security/keys/trusted.c
>> > index ddfaebf60fc8..d2b20b1cc2f1 100644
>> > --- a/security/keys/trusted.c
>> > +++ b/security/keys/trusted.c
>> > @@ -355,13 +355,12 @@ static int TSS_checkhmac2(unsigned char *buffer,
>> > * For key specific tpm requests, we will generate and send our
>> > * own TPM command packets using the drivers send function.
>> > */
>> > -static int trusted_tpm_send(const u32 chip_num, unsigned char *cmd,
>> > - size_t buflen)
>> > +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
>> > {
>> > int rc;
>> >
>> > dump_tpm_buf(cmd);
>> > - rc = tpm_send(chip_num, cmd, buflen);
>> > + rc = tpm_send(NULL, cmd, buflen);
>> > dump_tpm_buf(cmd);
>> > if (rc > 0)
>> > /* Can't return positive return codes values to keyctl */
>> > @@ -382,10 +381,10 @@ static int pcrlock(const int pcrnum)
>> >
>> > if (!capable(CAP_SYS_ADMIN))
>> > return -EPERM;
>> > - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
>> > + ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
>> > if (ret != SHA1_DIGEST_SIZE)
>> > return ret;
>> > - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
>> > + return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
>> > }
>> >
>> > /*
>> > @@ -398,7 +397,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
>> > unsigned char ononce[TPM_NONCE_SIZE];
>> > int ret;
>> >
>> > - ret = tpm_get_random(TPM_ANY_NUM, ononce, TPM_NONCE_SIZE);
>> > + ret = tpm_get_random(NULL, ononce, TPM_NONCE_SIZE);
>> > if (ret != TPM_NONCE_SIZE)
>> > return ret;
>> >
>> > @@ -410,7 +409,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
>> > store32(tb, handle);
>> > storebytes(tb, ononce, TPM_NONCE_SIZE);
>> >
>> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
>> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
>> > if (ret < 0)
>> > return ret;
>> >
>> > @@ -434,7 +433,7 @@ static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
>> > store16(tb, TPM_TAG_RQU_COMMAND);
>> > store32(tb, TPM_OIAP_SIZE);
>> > store32(tb, TPM_ORD_OIAP);
>> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
>> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
>> > if (ret < 0)
>> > return ret;
>> >
>> > @@ -493,7 +492,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
>> > if (ret < 0)
>> > goto out;
>> >
>> > - ret = tpm_get_random(TPM_ANY_NUM, td->nonceodd, TPM_NONCE_SIZE);
>> > + ret = tpm_get_random(NULL, td->nonceodd, TPM_NONCE_SIZE);
>> > if (ret != TPM_NONCE_SIZE)
>> > goto out;
>> > ordinal = htonl(TPM_ORD_SEAL);
>> > @@ -542,7 +541,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
>> > store8(tb, cont);
>> > storebytes(tb, td->pubauth, SHA1_DIGEST_SIZE);
>> >
>> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
>> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
>> > if (ret < 0)
>> > goto out;
>> >
>> > @@ -603,7 +602,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>> >
>> > ordinal = htonl(TPM_ORD_UNSEAL);
>> > keyhndl = htonl(SRKHANDLE);
>> > - ret = tpm_get_random(TPM_ANY_NUM, nonceodd, TPM_NONCE_SIZE);
>> > + ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE);
>> > if (ret != TPM_NONCE_SIZE) {
>> > pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
>> > return ret;
>> > @@ -635,7 +634,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>> > store8(tb, cont);
>> > storebytes(tb, authdata2, SHA1_DIGEST_SIZE);
>> >
>> > - ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE);
>> > + ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
>> > if (ret < 0) {
>> > pr_info("trusted_key: authhmac failed (%d)\n", ret);
>> > return ret;
>> > @@ -748,7 +747,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> > int i;
>> > int tpm2;
>> >
>> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
>> > + tpm2 = tpm_is_tpm2(NULL);
>> > if (tpm2 < 0)
>> > return tpm2;
>> >
>> > @@ -917,7 +916,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
>> > struct trusted_key_options *options;
>> > int tpm2;
>> >
>> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
>> > + tpm2 = tpm_is_tpm2(NULL);
>> > if (tpm2 < 0)
>> > return NULL;
>> >
>> > @@ -967,7 +966,7 @@ static int trusted_instantiate(struct key *key,
>> > size_t key_len;
>> > int tpm2;
>> >
>> > - tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
>> > + tpm2 = tpm_is_tpm2(NULL);
>> > if (tpm2 < 0)
>> > return tpm2;
>> >
>> > @@ -1008,7 +1007,7 @@ static int trusted_instantiate(struct key *key,
>> > switch (key_cmd) {
>> > case Opt_load:
>> > if (tpm2)
>> > - ret = tpm_unseal_trusted(TPM_ANY_NUM, payload, options);
>> > + ret = tpm_unseal_trusted(NULL, payload, options);
>> > else
>> > ret = key_unseal(payload, options);
>> > dump_payload(payload);
>> > @@ -1018,13 +1017,13 @@ static int trusted_instantiate(struct key *key,
>> > break;
>> > case Opt_new:
>> > key_len = payload->key_len;
>> > - ret = tpm_get_random(TPM_ANY_NUM, payload->key, key_len);
>> > + ret = tpm_get_random(NULL, payload->key, key_len);
>> > if (ret != key_len) {
>> > pr_info("trusted_key: key_create failed (%d)\n", ret);
>> > goto out;
>> > }
>> > if (tpm2)
>> > - ret = tpm_seal_trusted(TPM_ANY_NUM, payload, options);
>> > + ret = tpm_seal_trusted(NULL, payload, options);
>> > else
>> > ret = key_seal(payload, options);
>> > if (ret < 0)
>> > --
>> > 2.14.1
>> >
>>
>> Looks good to me. FWIW, Reviewed-by: PrasannaKumar Muralidharan
>> <[email protected]>.
>>
>> Regards,
>> PrasannaKumar
>
> Thank you.
>
> /Jarkko

I do not what value my rb tag provides as I have not contributed code
to it before. Is it encouraged by kernel community?

Thanks,
PrasannaKumar

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

2017-10-26 16:50:27

by Jason Gunthorpe

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Thu, Oct 26, 2017 at 09:57:12PM +0530, PrasannaKumar Muralidharan
wrote:

> I do not what value my rb tag provides as I have not contributed code
> to it before. Is it encouraged by kernel community?

Yes. People will judge the quality of your rb tag based on other
reviews they have seen you make, and here you found a bug, so very
good.

Got to start someplace.

Jason

2017-10-31 20:03:06

by Jason Gunthorpe

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Thu, Oct 26, 2017 at 03:54:50PM +0200, Jarkko Sakkinen wrote:
> Device number (the character device index) is not a stable identifier
> for a TPM chip. That is the reason why every call site passes
> TPM_ANY_NUM to tpm_chip_find_get().
>
> This commit changes the API in a way that instead a struct tpm_chip
> instance is given and NULL means the default chip. In addition, this
> commit refines the documentation to be up to date with the
> implementation.
>
> Suggested-by: Jason Gunthorpe <[email protected]> (@chip_num -> @chip)
> Signed-off-by: Jarkko Sakkinen <[email protected]>

Reviewed-by: Jason Gunthorpe <[email protected]>

Jason

2017-11-12 05:23:35

by PrasannaKumar Muralidharan

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

Did basic check on tpm rng patch, it works fine. As it depends on this
patch this should be working fine too.

Tested-by: PrasannaKumar Muralidharan <[email protected]>

Regards,
PrasannaKumar

2017-11-14 14:32:38

by Jarkko Sakkinen

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Sun, Nov 12, 2017 at 10:53:35AM +0530, PrasannaKumar Muralidharan wrote:
> Did basic check on tpm rng patch, it works fine. As it depends on this
> patch this should be working fine too.
>
> Tested-by: PrasannaKumar Muralidharan <[email protected]>
>
> Regards,
> PrasannaKumar

Thank you.

/Jarkko

2018-01-17 14:13:51

by PrasannaKumar Muralidharan

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

Hi Jarkko,

On 14 November 2017 at 20:02, Jarkko Sakkinen
<[email protected]> wrote:
> On Sun, Nov 12, 2017 at 10:53:35AM +0530, PrasannaKumar Muralidharan wrote:
>> Did basic check on tpm rng patch, it works fine. As it depends on this
>> patch this should be working fine too.
>>
>> Tested-by: PrasannaKumar Muralidharan <[email protected]>
>>
>> Regards,
>> PrasannaKumar
>
> Thank you.
>
> /Jarkko

Wondering what happened to this and tpm rng patch. Is there something
more to do for this to get merged?

Thanks,
PrasannaKumar

2018-01-18 16:20:14

by Jarkko Sakkinen

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

On Wed, Jan 17, 2018 at 07:43:51PM +0530, PrasannaKumar Muralidharan wrote:
> Hi Jarkko,
>
> On 14 November 2017 at 20:02, Jarkko Sakkinen
> <[email protected]> wrote:
> > On Sun, Nov 12, 2017 at 10:53:35AM +0530, PrasannaKumar Muralidharan wrote:
> >> Did basic check on tpm rng patch, it works fine. As it depends on this
> >> patch this should be working fine too.
> >>
> >> Tested-by: PrasannaKumar Muralidharan <[email protected]>
> >>
> >> Regards,
> >> PrasannaKumar
> >
> > Thank you.
> >
> > /Jarkko
>
> Wondering what happened to this and tpm rng patch. Is there something
> more to do for this to get merged?
>
> Thanks,
> PrasannaKumar

Was part of 4.6 PR.

/Jarkko

2018-01-20 08:00:12

by PrasannaKumar Muralidharan

Subject: Re: [PATCH v3] tpm: use struct tpm_chip for tpm_chip_find_get()

Hi Jarkko,

On 18 January 2018 at 21:50, Jarkko Sakkinen
<[email protected]> wrote:
> On Wed, Jan 17, 2018 at 07:43:51PM +0530, PrasannaKumar Muralidharan wrote:
>> Hi Jarkko,
>>
>> On 14 November 2017 at 20:02, Jarkko Sakkinen
>> <[email protected]> wrote:
>> > On Sun, Nov 12, 2017 at 10:53:35AM +0530, PrasannaKumar Muralidharan wrote:
>> >> Did basic check on tpm rng patch, it works fine. As it depends on this
>> >> patch this should be working fine too.
>> >>
>> >> Tested-by: PrasannaKumar Muralidharan <[email protected]>
>> >>
>> >> Regards,
>> >> PrasannaKumar
>> >
>> > Thank you.
>> >
>> > /Jarkko
>>
>> Wondering what happened to this and tpm rng patch. Is there something
>> more to do for this to get merged?
>>
>> Thanks,
>> PrasannaKumar
>
> Was part of 4.6 PR.
>
> /Jarkko

Ah, I see.

PS: I think you meant 4.16.

Thanks,
PrasannaKumar