Stefan Berger <stef...@linux.ibm.com> writes:
> On 7/14/21 12:16 PM, Daniel Kiper wrote:
>> CC-ing folks CC-ed in Daniel's patch series and Eric.
>>
>> On Mon, Jul 12, 2021 at 03:02:19PM -0400, Stefan Berger wrote:
>>> From: Stefan Berger <stef...@linux.ibm.com>
>>>
>>> Add support for trusted boot using a vTPM 2.0 on the IBM ieee1275
>>> platform. With this patch grub now measures text and binary data
>>> into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
>>> does.
>>>
>>> This patch requires Daniel Axtens's patches for claiming more memory.
>> Would not be it simpler if you take out the memory mgmt changes patches
>> from Daniel's patch series?
> You mean reposting this series with his 3 patches upfront? I can
> certainly do that.
>>
>> Daniel, are you OK with it?
>>
I don't mind what series the patches go through, as long as they get
merged at some point :)
Kind regards,
Daniel
>> Additionally, please CC Eric when you post IEEE1275 patches next time.
>
>
> Will do.
>
>
>>
>>> Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
>>> ---
>>> grub-core/Makefile.core.def | 8 ++
>>> grub-core/commands/ieee1275/ibmvtpm.c | 118 ++++++++++++++++++++++++++
>>> grub-core/kern/ieee1275/ibmvtpm.c | 62 ++++++++++++++
>>> include/grub/ieee1275/ibmvtpm.h | 32 +++++++
>>> 4 files changed, 220 insertions(+)
>>> create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
>>> create mode 100644 grub-core/kern/ieee1275/ibmvtpm.c
>>> create mode 100644 include/grub/ieee1275/ibmvtpm.h
>>>
>>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>>> index 3f3459b2c..e2a64f8ff 100644
>>> --- a/grub-core/Makefile.core.def
>>> +++ b/grub-core/Makefile.core.def
>>> @@ -1166,6 +1166,14 @@ module = {
>>> enable = powerpc_ieee1275;
>>> };
>>>
>>> +module = {
>>> + name = tpm;
>>> + common = commands/tpm.c;
>>> + common = kern/ieee1275/ibmvtpm.c;
>>> + ieee1275 = commands/ieee1275/ibmvtpm.c;
>> s/ieee1275/common/?
>>
>>> + enable = powerpc_ieee1275;
>>> +};
>>> +
>>> module = {
>>> name = terminal;
>>> common = commands/terminal.c;
>>> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c
>>> b/grub-core/commands/ieee1275/ibmvtpm.c
>>> new file mode 100644
>>> index 000000000..9b06c76d9
>>> --- /dev/null
>>> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
>>> @@ -0,0 +1,118 @@
>>> +/*
>>> + * GRUB -- GRand Unified Bootloader
>>> + * Copyright (C) 2021 IBM Corporation
>> Copyright (C) 2021 Free Software Foundation, Inc.
>>
>> Or both if you strongly need IBM...
>>
>>> + * GRUB is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 3 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * GRUB is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
>>> + *
>>> + * IBM vTPM support code.
>>> + */
>>> +
>>> +#include <grub/err.h>
>>> +#include <grub/types.h>
>>> +#include <grub/tpm.h>
>>> +#include <grub/ieee1275/ieee1275.h>
>>> +#include <grub/ieee1275/ibmvtpm.h>
>>> +#include <grub/mm.h>
>>> +#include <grub/misc.h>
>>> +
>>> +static grub_ieee1275_ihandle_t grub_tpm_ihandle;
>>> +static grub_uint8_t grub_tpm_version;
>>> +
>>> +static void grub_ieee1275_tpm_init (grub_ieee1275_ihandle_t *tpm_ihandle)
>> You can drop from static functions, variables, etc. names "grub_" prefix.
>>
>>> +{
>>> + static int init_called = 0;
>>> +
>>> + if (!init_called) {
>>> + init_called = 1;
>>> + grub_ieee1275_open ("/vdevice/vtpm", &grub_tpm_ihandle);
>>> + }
>> Incorrect formatting. It should be
>>
>> if (!init_called)
>> {
>> init_called = 1;
>> grub_ieee1275_open ("/vdevice/vtpm", &grub_tpm_ihandle);
>> }
>>
>> You can always refer to grub-core/kern/efi/sb.c to get formatting
>> examples for various pieces of the GRUB code.
>>
>> Please fix similar issues below too.
>>
>>> + *tpm_ihandle = grub_tpm_ihandle;
>>> +}
>>> +
>>> +static grub_err_t
>>> +grub_tpm_get_tpm_version (grub_uint8_t *protocol_version)
>>> +{
>>> + static int version_probed = 0;
>>> + grub_ieee1275_phandle_t vtpm;
>>> + char buffer[20];
>>> + grub_ssize_t buffer_size;
>>> +
>>> + if (!version_probed) {
>>> + version_probed = 1;
>>> + if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
>>> + !grub_ieee1275_get_property (vtpm, "compatible", buffer,
>>> + sizeof buffer, &buffer_size) &&
>>> + !grub_strcmp (buffer, "IBM,vtpm20")) {
>>> + grub_tpm_version = 2;
>>> + }
>>> + }
>>> + *protocol_version = grub_tpm_version;
>>> +
>>> + return 0;
>> You ignore this value later.
>>
>> I think the prototype of this function should be following:
>> grub_uint8_t get_tpm_version (void)
>>
>> Which means you should return the TPM version.
>>
>>> +}
>>> +
>>> +static grub_int8_t
>> static grub_err_t
>>
>>> +grub_tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle,
>>> + grub_uint8_t *protocol_version)
>>> +{
>>> + grub_ieee1275_tpm_init (tpm_handle);
>>> + if (*tpm_handle == NULL)
>>> + return 0;
>> return GRUB_ERR_UNKNOWN_DEVICE;
>>
>>> + grub_tpm_get_tpm_version (protocol_version);
>>> +
>>> + return 1;
>> return GRUB_ERR_NONE;
>>
>>> +}
>>> +
>>> +static grub_err_t
>>> +grub_tpm2_log_event (grub_ieee1275_ihandle_t tpm_handle, unsigned char
>>> *buf,
>>> + grub_size_t size, grub_uint8_t pcr,
>>> + const char *description)
>>> +{
>>> + static int error_displayed;
>>> + bool succ;
>> s/bool/grub_err_t/
>>
>>> +
>>> + succ = grub_ieee1275_ibmvtpm_2hash_ext_log (tpm_handle,
>>> + pcr, EV_IPL,
>>> + description,
>>> + grub_strlen(description) + 1,
>>> + buf, size);
>>> + if (!succ && !error_displayed) {
>>> + error_displayed = 1;
>>> + grub_printf("2HASH-EXT-LOG failed: Firmware is likely too old.\n");
>> s/grub_printf/grub_error/
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +grub_err_t
>>> +grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
>>> + const char *description)
>>> +{
>>> + grub_ieee1275_ihandle_t tpm_handle;
>>> + grub_uint8_t protocol_version = 0;
>> It seems to me that you have the same information in the grub_tpm_version
>> global variable. So, I think you should you use it and drop all instances
>> of protocol_version.
>>
>>> + /* Absence of a TPM isn't a failure. */
>>> + if (!grub_tpm_handle_find (&tpm_handle, &protocol_version))
>>> + return 0;
>>> +
>>> + grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ",
>>> %s\n",
>>> + pcr, size, description);
>>> +
>>> + if (protocol_version == 2)
>>> + return grub_tpm2_log_event (tpm_handle, buf, size, pcr, description);
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/grub-core/kern/ieee1275/ibmvtpm.c
>>> b/grub-core/kern/ieee1275/ibmvtpm.c
>>> new file mode 100644
>>> index 000000000..525a792b4
>>> --- /dev/null
>>> +++ b/grub-core/kern/ieee1275/ibmvtpm.c
>>> @@ -0,0 +1,62 @@
>>> +/* ibmvtpm.c - Client interface to access the IBM vTPM */
>>> +/*
>>> + * GRUB -- GRand Unified Bootloader
>>> + * Copyright (C) 2021 IBM Corporation
>>> + *
>>> + * GRUB is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation, either version 3 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * GRUB is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <grub/types.h>
>>> +#include <grub/misc.h>
>>> +#include <grub/ieee1275/ieee1275.h>
>>> +#include <grub/ieee1275/ibmvtpm.h>
>>> +
>>> +bool
>> s/bool/grub_err_t/
>>
>>> +grub_ieee1275_ibmvtpm_2hash_ext_log (grub_uint32_t ihandle,
>>> + grub_uint8_t pcrindex,
>>> + grub_uint32_t eventtype,
>>> + const char *description,
>>> + grub_size_t description_size,
>>> + void *buf, grub_size_t size)
>> I cannot see any reason to make this function part of the GRUB kernel.
>> I think it should be in grub-core/commands/ieee1275/ibmvtpm.c.
>>
>>> +{
>>> + struct tpm_2hash_ext_log
>> Please define this struct before the function, somewhere after includes.
>>
>>> + {
>>> + struct grub_ieee1275_common_hdr common;
>>> + grub_ieee1275_cell_t method;
>>> + grub_ieee1275_cell_t ihandle;
>>> + grub_ieee1275_cell_t size;
>>> + void *buf;
>>> + grub_ieee1275_cell_t description_size;
>>> + const char *description;
>>> + grub_ieee1275_cell_t eventtype;
>>> + grub_ieee1275_cell_t pcrindex;
>>> + grub_ieee1275_cell_t catch_result;
>>> + grub_ieee1275_cell_t rc;
>>> + }
>> GRUB_PACKED?
>>
>>> + args;
>>> +
>>> + INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
>>> + args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
>>> + args.ihandle = ihandle;
>>> + args.pcrindex = pcrindex;
>>> + args.eventtype = eventtype;
>>> + args.description = description;
>>> + args.description_size = description_size;
>>> + args.buf = buf;
>>> + args.size = (grub_ieee1275_cell_t) size;
>>> +
>>> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>>> + return false;
>>> + return !!args.rc;
>>> +}
>> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
