On Tue Aug 22, 2023 at 2:15 PM EEST, Jarkko Sakkinen wrote:
> On Tue Apr 4, 2023 at 12:39 AM EEST, James Bottomley wrote:
> > Extracting values from returned TPM buffers can be hard. Add cursor
> > based (moving poiner) functions that make it easier to extract TPM
> > returned values from a buffer.
> >
> > Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> >
> > ---
> > v4: add kernel doc and reword commit
> > ---
> > drivers/char/tpm/tpm-buf.c | 48 ++++++++++++++++++++++++++++++++++++++
> > include/linux/tpm.h | 3 +++
> > 2 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index b7e42fb6266c..da0f6e725c3f 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
> > @@ -6,6 +6,8 @@
> > #include <linux/module.h>
> > #include <linux/tpm.h>
> >
> > +#include <asm/unaligned.h>
> > +
> > static int __tpm_buf_init(struct tpm_buf *buf)
> > {
> > buf->data = (u8 *)__get_free_page(GFP_KERNEL);
> > @@ -229,3 +231,49 @@ void tpm_buf_append_2b(struct tpm_buf *buf, struct tpm_buf *tpm2b)
> > tpm_buf_reset_int(tpm2b);
> > }
> > EXPORT_SYMBOL_GPL(tpm_buf_append_2b);
> > +
> > +/* functions for unmarshalling data and moving the cursor */
> > +
> > +/**
> > + * tpm_get_inc_u8 - read a u8 and move pointer beyond it
> > + * @ptr: pointer to pointer
> > + *
> > + * @return: value read
> > + */
> > +u8 tpm_get_inc_u8(const u8 **ptr)
> > +{
> > + return *((*ptr)++);
> > +}
> > +EXPORT_SYMBOL_GPL(tpm_get_inc_u8);
>
> No overflow check, and these should be static inlines.
>
> Please consider this:
>
> /**
> * tpm_buf_read_u8() - Read a byte from a TPM buffer
> * @buf: &tpm_buf instance
> * @offset: offset within the consumed part of the buffer
> */
> static inline int tpm_buf_read_u8(const struct tpm_buf *buf, offs_t *offset)
> {
> if (*offset++ >= buf->length)
Oops, this increases pointer location, not value, sorry (should be (*offset)++).
> return -EINVAL;
>
> return buf->data[*offset - 1];
> }
Actually probably best would be if you would add first (in order to
have all the logic in a single location):
/**
* tpm_buf_read() - Read from a TPM buffer
* @buf: &tpm_buf instance
* @count: the number of bytes to read
* @offset: offset within the buffer
* @output: the output buffer
*/
int tpm_buf_read(const struct tpm_buf *buf, size_t count, offs_t *offset, void *output)
{
if (*(offset + count) >= buf->length)
return -EINVAL;
memcpy(output, &buf->data[*offset], count);
*offset += count;
] return 0;
}
EXPORT_SYMBOL_GPL(tpm_buf_read);
For instance:
static inline static int tpm_buf_read_u16(const struct tpm_buf *buf, offs_t *offset)
{
u16 value;
int ret;
ret = tpm_buf_read(buf, sizeof(u16), offset, &value);
return ret ? ret : be16_to_cpu(value);
}
BR, Jarkko
