Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:
> +keyctl print returns an ascii hex copy of the sealed key, which is in
> standard
I'd quote 'keyctl print' just so it's obvious where the command ends and the
descriptive text starts.
> +Usage:
> + keyctl add encrypted name "new key-type:master-key-name keylen" ring
> + keyctl add encrypted name "load key-type:master-key-name keylen hex_blob"
> ring
> + keyctl update keyid "update key-type:master-key-name"
> +
> +where 'key-type' is either 'trusted' or 'user'.
I recommend adding some example commands with all the arguments substituted.
Nothing helps get to grip with an API like knowing what a command is supposed
to look like when it's actually used.
> +static int trusted_tpm_send(u32 chip_num, unsigned char *cmd, int buflen)
There are still a lot of places in here where you should probably be using
const and size_t.
> +static int my_get_random(unsigned char *buf, int len)
> +{
> + struct tpm_buf *tb;
> + int ret;
> +
> + tb = kzalloc(sizeof *tb, GFP_KERNEL);
> + if (!tb)
> + return -ENOMEM;
> + ret = tpm_get_random(tb, buf, len);
Using kzalloc() rather than kmalloc() is a waste of time, I'd've thought.
It's a temporary buffer. Does it really need to be precleared?
> + ret = tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash);
> + return ret;
Merge.
> +static int trusted_update(struct key *key, const void *data, size_t datalen)
> +{
> ...
> + *(datablob + datalen) = '\0';
That's what [] is for.
> + if (new_o)
> + kfree(new_o);
kfree() can handle a NULL pointer.
> + if (new_o->pcrlock)
> + ret = pcrlock(new_o->pcrlock);
> + rcu_assign_pointer(key->payload.data, new_p);
> + call_rcu(&p->rcu, trusted_rcu_free);
Should there be a check for pcrlock() failure?
> +/* not already defined in tpm.h - specific to this use */
> +#define TPM_TAG_RQU_COMMAND 193
> +#define TPM_TAG_RQU_AUTH1_COMMAND 194
> ...
Values defined for TPM hardware access really ought to be in a separate file
in include/linux/. They aren't strictly specific to the trusted key
implementation here; that may be the only user currently in the kernel, but
that doesn't mean there can't be another user.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
