Re: [PATCH 06/10] tpm: introduce tpm_buf

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Sun, Oct 18, 2015 at 04:57:49AM +0200, Peter Hüwe wrote:
> Am Freitag, 16. Oktober 2015, 20:40:25 schrieb Jarkko Sakkinen:
> > This patch introduces struct tpm_buf that provides a string buffer for
> > constructing TPM commands. This allows to construct variable sized TPM
> > commands. For the buffer a page is allocated and mapped, which limits
> > maximum size to PAGE_SIZE.
> > 
> > Variable sized TPM commands are needed in order to add algorithmic
> > agility.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  drivers/char/tpm/tpm.h | 97
> > ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97
> > insertions(+)
> > 
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 36ceb71..cb46f62 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -1,5 +1,6 @@
> >  /*
> >   * Copyright (C) 2004 IBM Corporation
> > + * Copyright (C) 2015 Intel Corporation
> >   *
> >   * Authors:
> >   * Leendert van Doorn <leendert@watson.ibm.com>
> > @@ -28,6 +29,7 @@
> >  #include <linux/tpm.h>
> >  #include <linux/acpi.h>
> >  #include <linux/cdev.h>
> > +#include <linux/highmem.h>
> > 
> >  enum tpm_const {
> >  	TPM_MINOR = 224,	/* officially assigned */
> > @@ -390,6 +392,101 @@ struct tpm_cmd_t {
> >  	tpm_cmd_params	params;
> >  } __packed;
> > 
> > +/* A string buffer type for constructing TPM commands. This is based on
> > the + * ideas of string buffer code in security/keys/trusted.h but is heap
> > based + * in order to keep the stack usage minimal.
> > + */
> > +
> > +enum tpm_buf_flags {
> > +	TPM_BUF_OVERFLOW	= BIT(0),
> > +};
> > +
> > +struct tpm_buf {
> > +	struct page *data_page;
> > +	unsigned int flags;
> > +	u8 *data;
> > +};
> > +
> > +static inline void tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> > +{
> > +	struct tpm_input_header *head;
> > +
> > +	buf->data_page = alloc_page(GFP_HIGHUSER);
> > +	if (!buf->data_page)
> > +		return -ENOMEM;
> > +
> > +	buf->flags = 0;
> > +	buf->data = kmap(buf->data_page);
> > +
> > +	head = (struct tpm_input_header *) buf->data;
> > +
> > +	head->tag = cpu_to_be16(tag);
> > +	head->length = cpu_to_be32(sizeof(*head));
> > +	head->ordinal = cpu_to_be32(ordinal);
> > +
> > +	return 0;
> > +}
> > +
> > +static inline void tpm_buf_destroy(struct tpm_buf *buf)
> > +{
> > +	kunmap(buf->data_page);
> > +	__free_page(buf->data_page);
> > +}
> > +
> > +static inline u32 tpm_buf_length(struct tpm_buf *buf)
> > +{
> > +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> > +
> > +	return be32_to_cpu(head->length);
> > +}
> > +
> > +static inline u16 tpm_buf_tag(struct tpm_buf *buf)
> > +{
> > +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> > +
> > +	return be16_to_cpu(head->tag);
> > +}
> > +
> > +static inline void tpm_buf_append(struct tpm_buf *buf,
> > +				  const unsigned char *new_data,
> > +				  unsigned int new_len)
> > +{
> > +	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
> > +	u32 len = tpm_buf_length(buf);
> 
> 
> > +
> > +	/* Return silently if overflow has already happened. */
> > +	if (buf->flags & TPM_BUF_OVERFLOW)
> > +		return;
> > +
> > +	if ((len + new_len) > PAGE_SIZE) {
> > +		WARN(1, "tpm_buf: overflow\n");
> > +		buf->flags |= TPM_BUF_OVERFLOW;
> > +		return;
> > +	}
> Why not use WARN_ONCE?

Does it matter? Message is emitted only once since it does not sense to
even try to add new data if overflow flag has been set. I.e. even if I
had WARN_ONCE there I still would have the check for overflow flag
before this.

/Jarkko