On Fri, 2010-11-12 at 16:52 +0000, David Howells wrote:
> Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
Again, thanks for the detailed review!
Willdo on all suggestions with a couple of comments/questions:
> > +#define TPM_MAX_BUF_SIZE 512
> > +#define TPM_TAG_RQU_COMMAND 193
> > +#define TPM_TAG_RQU_AUTH1_COMMAND 194
> > +#define TPM_TAG_RQU_AUTH2_COMMAND 195
> > +#define TPM_TAG_RSP_COMMAND 196
> > +#define TPM_TAG_RSP_AUTH1_COMMAND 197
> > +#define TPM_TAG_RSP_AUTH2_COMMAND 198
> > +#define TPM_NONCE_SIZE 20
> > +#define TPM_HASH_SIZE 20
> > +#define TPM_SIZE_OFFSET 2
> > +#define TPM_RETURN_OFFSET 6
> > +#define TPM_DATA_OFFSET 10
> > +#define TPM_U32_SIZE 4
> > +#define TPM_GETRANDOM_SIZE 14
> > +#define TPM_GETRANDOM_RETURN 14
> > +#define TPM_ORD_GETRANDOM 70
> > +#define TPM_RESET_SIZE 10
> > +#define TPM_ORD_RESET 90
> > +#define TPM_OSAP_SIZE 36
> > +#define TPM_ORD_OSAP 11
> > +#define TPM_OIAP_SIZE 10
> > +#define TPM_ORD_OIAP 10
> > +#define TPM_SEAL_SIZE 87
> > +#define TPM_ORD_SEAL 23
> > +#define TPM_ORD_UNSEAL 24
> > +#define TPM_UNSEAL_SIZE 104
> > +#define SEALKEYTYPE 1
> > +#define SRKKEYTYPE 4
> > +#define SRKHANDLE 0x40000000
> > +#define TPM_ANY_NUM 0xFFFF
> > +#define MAX_PCRINFO_SIZE 64
>
> I suspect some of these should be in somewhere like linux/tpm.h rather than
> here. They're specific to TPM access not TPM key management.
Most of them are not already defined in tpm.h, as they are never used
there, but you are right, some are generic. I'll double check these..
> > +#define TPM_DEBUG 0
>
> The TPM_DEBUG stuff should probably be in the directory with the sources, not
> in a directory for others to include.
Maybe some confusion here - trusted_defined.h is in the sources - only
trusted-type.h is public in include/keys/.
> > +#if TPM_DEBUG
> > +static inline void dump_options(struct trusted_key_options *o)
> > +{
> > + pr_info("trusted_key: sealing key type %d\n", o->keytype);
> > + pr_info("trusted_key: sealing key handle %0X\n", o->keyhandle);
> > + pr_info("trusted_key: pcrlock %d\n", o->pcrlock);
> > + pr_info("trusted_key: pcrinfo %d\n", o->pcrinfo_len);
> > + print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> > + 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> > +}
> > +
...
> > +
> > +static inline void store16(struct tpm_buf *buf, uint16_t value)
> > +{
> > + *(uint16_t *) & buf->data[buf->len] = htons(value);
> > + buf->len += sizeof(value);
> > +}
> > +
> > +static inline void store32(struct tpm_buf *buf, uint32_t value)
> > +{
> > + *(uint32_t *) & buf->data[buf->len] = htonl(value);
> > + buf->len += sizeof(value);
> > +}
> > +
> > +static inline void storebytes(struct tpm_buf *buf, unsigned char *in, int len)
> > +{
> > + memcpy(buf->data + buf->len, in, len);
> > + buf->len += len;
> > +}
>
> Also these look like internal functions which shouldn't be in the global
> headers.
This is in trusted_defined.h, which is with sources, not in
include/keys/
thanks!
dave
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
