[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080528001739.54c630f9.akpm@linux-foundation.org>
Date: Wed, 28 May 2008 00:17:39 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Mimi Zohar <zohar@...ux.vnet.ibm.com>
Cc: linux-kernel@...r.kernel.org, safford@...son.ibm.com,
serue@...ux.vnet.ibm.com, sailer@...son.ibm.com, zohar@...ibm.com,
Stephen Smalley <sds@...ho.nsa.gov>,
CaseySchaufler <casey@...aufler-ca.com>,
Harvey Harrison <harvey.harrison@...il.com>
Subject: Re: [RFC][Patch 2/5]integrity: TPM internel kernel interface
On Fri, 23 May 2008 11:03:41 -0400 Mimi Zohar <zohar@...ux.vnet.ibm.com> wrote:
> Resubmitting integrity-tpm-internal-kernel-interface.patch, which
> was previously Signed-off-by Kylene Hall.
>
> Adds the following support:
> - make internal kernel interface to transmit TPM commands global
> - adds reading a pcr value
> - adds extending a pcr value
> - adds lookup the tpm_chip for given chip number and type
>
> Signed-off-by: Mimi Zohar <zohar@...ibm.com>
Is this effort a once-off, or should you add yourself to ./MAINTAINERS?
> ...
>
> +/*
> + * tpm_chip_lookup - return tpm_chip for given chip number and type
> + */
> +static struct tpm_chip *tpm_chip_lookup(int chip_num, int chip_typ)
> +{
> + struct tpm_chip *pos;
> +
> + spin_lock(&driver_lock);
> + list_for_each_entry(pos, &tpm_chip_list, list)
> + if ((chip_num == TPM_ANY_NUM || pos->dev_num == chip_num)
hard tabs for indenting here, please.
> + && (chip_typ == TPM_ANY_TYPE)) {
> + spin_unlock(&driver_lock);
> + return pos;
> + }
> +
> + spin_unlock(&driver_lock);
> + return NULL;
> +}
> +
> +/**
> + * tpm_pcr_read - read a pcr value
> + * @chip_id: tpm chip identifier
> + * Upper 2 bytes: ANY, HW_ONLY or SW_ONLY
> + * Lower 2 bytes: tpm idx # or AN&
> + * @pcr_idx: pcr idx to retrieve
> + * @res_buf: TPM_PCR value
> + * @res_buf_size: 20 bytes (or NULL if you don't care)
> + */
> +int tpm_pcr_read(u32 chip_id, int pcr_idx, u8 *res_buf, int res_buf_size)
> +{
> + u8 data[READ_PCR_RESULT_SIZE];
> + int rc;
> + __be32 index;
> + int chip_num = chip_id & TPM_CHIP_NUM_MASK;
> + struct tpm_chip *chip;
> +
> + if (res_buf && res_buf_size < TPM_DIGEST_SIZE)
> + return -ENOSPC;
> +
> + chip = tpm_chip_lookup(chip_num, chip_id >> TPM_CHIP_TYPE_SHIFT);
> + if (chip == NULL)
> + return -ENODEV;
> +
> + memcpy(data, pcrread, sizeof(pcrread));
For robustness purposes it would be good to have a check that
sizeof(pcrread) does not exceed READ_PCR_RESULT_SIZE. One way of doing
that would be to add
if (sizeof(pcrread) > READ_PCR_RESULT_SIZE)
function_which_does_not_exist();
under the assumption that gcc will not emit the call. Or a plain old
BUG_ON() in the init code somewhere.
Or, better, implement all of this in a typesafe manner, using the C
type system. Is that possible? Things like unions mght be needed...
> + index = cpu_to_be32(pcr_idx);
> + memcpy(data + 10, &index, 4);
> + rc = tpm_transmit(chip, data, sizeof(data));
> + if (rc > 0)
> + rc = be32_to_cpu(*((u32 *) (data + 6)));
An unaligned access. What architectures is this hardware available on?
Harvey might be able to suggest a neater and better way of doing this?
At least a get_unaligned(), I think?
> +
> + if (rc == 0 && res_buf)
> + memcpy(res_buf, data + 10, TPM_DIGEST_SIZE);
> +
> + return rc;
> +
> +}
> +EXPORT_SYMBOL_GPL(tpm_pcr_read);
> +
> +#define EXTEND_PCR_SIZE 34
> +static const u8 pcrextend[] = {
> + 0, 193, /* TPM_TAG_RQU_COMMAND */
> + 0, 0, 0, 34, /* length */
> + 0, 0, 0, 20, /* TPM_ORD_Extend */
> + 0, 0, 0, 0 /* PCR index */
> +};
> +
> +/**
> + * tpm_pcr_extend - extend pcr value with hash
> + * @chip_id: tpm chip identifier
> + * Upper 2 bytes: ANY, HW_ONLY or SW_ONLY
> + * Lower 2 bytes: tpm idx # or AN&
> + * @pcr_idx: pcr idx to extend
> + * @hash: hash value used to extend pcr value
> + */
> +int tpm_pcr_extend(u32 chip_id, int pcr_idx, const u8 *hash)
> +{
> + u8 data[EXTEND_PCR_SIZE];
> + int rc;
> + __be32 index;
> + int chip_num = chip_id & TPM_CHIP_NUM_MASK;
> + struct tpm_chip *chip;
> +
> + chip = tpm_chip_lookup(chip_num, chip_id >> TPM_CHIP_TYPE_SHIFT);
> + if (chip == NULL)
> + return -ENODEV;
> +
> + memcpy(data, pcrextend, sizeof(pcrextend));
> + index = cpu_to_be32(pcr_idx);
> + memcpy(data + 10, &index, 4);
> + memcpy(data + 14, hash, TPM_DIGEST_SIZE);
> + rc = tpm_transmit(chip, data, sizeof(data));
> + if (rc > 0)
> + rc = be32_to_cpu(*((u32 *) (data + 6)));
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_pcr_extend);
Dittoes.
>
> ...
>
> +#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
> +
> +extern int tpm_pcr_read(u32 chip_id, int pcr_idx, u8 *res_buf,
> + int res_buf_size);
> +extern int tpm_pcr_extend(u32 chip_id, int pcr_idx, const u8 *hash);
> +#else
> +static inline int tpm_pcr_read(u32 chip_id, int pcr_idx, u8 *res_buf,
> + int res_buf_size)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int tpm_pcr_extend(u32 chip_id, int pcr_idx, const u8 *hash)
> +{
> + return -ENODEV;
> +}
> +#endif
> +#endif
Are the !CONFIG_TCG_TPM stub functions actually needed? Perhaps all
the code which can call tpm_pcr_read() and tpm_pcr_extend() cannot be
coppiled when CONFIG_TCG_TPM=n due to Kconfig dependencies?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists