Re: [PATCH v2 1/3] efi_loader: add SMBIOS table measurement

[Masahisa Kojima <masahisa.kojima@linaro.org>]

On Thu, 23 Sept 2021 at 01:19, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Masahisa,
>
> On Tue, 21 Sept 2021 at 01:17, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
> >
> > TCG PC Client spec requires to measure the SMBIOS
> > table that contain static configuration information
> > (e.g. Platform Manufacturer Enterprise Number assigned by IANA,
> > platform model number, Vendor and Device IDs for each SMBIOS table).
> >
> > The device and environment dependent information such as
>
> device- and environment-dependent
>
> > serial number is cleared to zero or space character for
> > the measurement.
> >
> > Existing smbios_string() function returns pointer to the string
> > with const qualifier, but exisintg use case is updating version
> > string and const qualifier must be removed.
> > This commit removes const qualifier from smbios_string()
> > return value and reuses to clear the strings for the measurement.
> >
> > This commit also fixes the following compiler warning:
> >
> > lib/smbios-parser.c:59:39: warning: cast to pointer from integer of
> > different size [-Wint-to-pointer-cast]
> >   const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address;
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> >
> > Changes in v2:
> > - use flexible array for table_entry field
> > - modify funtion name to find_smbios_table()
> > - remove unnecessary const qualifier from smbios_string()
> > - create non-const version of next_header()
> >
> >  include/efi_loader.h          |   2 +
> >  include/efi_tcg2.h            |  15 ++++
> >  include/smbios.h              |  17 +++-
> >  lib/efi_loader/Kconfig        |   1 +
> >  lib/efi_loader/efi_boottime.c |   2 +
> >  lib/efi_loader/efi_smbios.c   |   2 -
> >  lib/efi_loader/efi_tcg2.c     |  84 +++++++++++++++++++
> >  lib/smbios-parser.c           | 152 +++++++++++++++++++++++++++++++---
> >  8 files changed, 261 insertions(+), 14 deletions(-)
>
> Where are the tests for this new code, please?
>
> Would it make sense to have a function that iterates through the data
> that does need to be hashed, instead?

I agree that it is straightforward if we iterates the data to be hashed.
But the data NOT to be hashed is less than the data to be hashed,
and this is what edk2 implements, we can easily compare with edk2.
So I would like to keep current implementation.

Thanks,
Masahisa Kojima

>
> Regards,
> Simon