Re: [PATCH v6 3/6] tpm: Support boot measurements

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Eddie,

[...]

> > But at least the kernel can find the EFI config tables properly.
> > > Also, note that the ret is efi_status_t and the new function you added
> > > returns an int.  Just switch the ret here accordingly.
> > >
> > >
> > >           * platforms can use different ways to do so.
> > >           */
> > >          ret = tcg2_init_log(dev, &elog);
> > > -       if (ret != EFI_SUCCESS)
> > > -               goto free_pool;
> > > -
> > > -       memcpy(event_log.buffer, elog.log, elog.log_position);
> > > -       event_log.pos = elog.log_position;
> > > +       if (ret == EFI_SUCCESS) {
> > > +               memcpy(event_log.buffer, elog.log, elog.log_position);
> > > +               event_log.pos = elog.log_position;
> > > +       }
> > >
> > >
> > > I'll try digging into the EFI issues more once I get some time
> > Ok Ignore the previous mail. I just noticed that the specid event is
> > generated by tcg2_log_init() and that's where the errors come from.
>
>
> OK, I think I understand the problem. One problem with your solution below
> is that it's only useful for EFI; writing the event log to a temporary
> buffer in the non-EFI case is a waste of time, since it can't go anywhere.

It's not even a 'solution', it's just an ugly hack to help you understand
where things go sideways.

>
> I have a solution where the user can pass in their buffer in the
> tcg2_event_log structure in the event that no memory region is discovered.
> EFI can use that path.

Yep that sounds sane,  that way EFI can pass the efi allocated memory and
you can call call that with NULL for non-efi code.
Please keep in mind that the EventLog, if it comes from TF-A, needs to be
replayed into hardware as well, since TF-A doesn't extend PCRs.

>
>
> Thanks for the debugging effort!

yw

Cheers
/Ilias
>
> Eddie
>
>
> >
> > So something like the *really ugly hack* fixes EFI.  Can you please
> > turn it into human-readable code
> >
> >   #include <linux/unaligned/be_byteshift.h>
> >   #include <linux/unaligned/generic.h>
> >   #include <linux/unaligned/le_byteshift.h>
> > +#include <malloc.h>
> >
> >   #include "tpm-utils.h"
> >
> > @@ -634,7 +635,7 @@ __weak int tcg2_platform_get_log(struct udevice
> > *dev, void **addr, u32 *size)
> >
> >                  if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0,
> >                                                 0, &args))
> > -                       return -ENODEV;
> > +                       goto alloc;
> >
> >                  a = ofnode_get_addr_size(args.node, "reg", &s);
> >                  if (a == FDT_ADDR_T_NONE)
> > @@ -643,6 +644,9 @@ __weak int tcg2_platform_get_log(struct udevice
> > *dev, void **addr, u32 *size)
> >                  *addr = map_physmem(a, s, MAP_NOCACHE);
> >                  *size = (u32)s;
> >          }
> > +alloc:
> > +       *addr = calloc(1, 4096);
> > +       *size = 4096;
> >
> >          return 0;
> >   }
> >
> > Cheers
> > /Ilias
> >
> > > Cheers
> > > /Ilias
> > > > Thanks!
> > > > /Ilias