> On 17 Jul 2025, at 7:25 PM, Daniel Kiper <dki...@net-space.pl> wrote:
>
> On Mon, Jul 14, 2025 at 11:05:07PM +0530, Sudhakar Kuppusamy wrote:
>> Enhancing the infrastructure to enable the Platform Keystore (PKS) feature,
>> which provides access to the SB VERSION, DB, and DBX secure boot variables
>> from PKS.
>>
>> If secure boot is enabled with PKS, it will read secure boot variables
>> such as db and dbx from PKS and extract ESL's from it.
>> The ESL's would be saved in the platform keystore buffer, and
>> the appendedsig (module) would read it later to extract
>> the certificate's details from ESL.
>>
>> In the following scenarios, static key mode will be activated:
>> 1. When Secure Boot is enabled with static keys
>> 2. When SB Version is unavailable but Secure Boot is enabled
>> 3. When PKS support is unavailable but Secure Boot is enabled
>>
>> Note:-
>>
>> SB Version - Secure Boot mode
>> 1 - PKS
>> 0 - static key (embeded key)
>
> I think "static key" mode should be explained in the commit message too.
> Additionally, I think it is worth explaining key differences between two
> modes. I do not mention these should be put into GRUB documentation…
Will do it.
>
>> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
>> ---
>> grub-core/Makefile.am | 2 +
>> grub-core/Makefile.core.def | 2 +
>> grub-core/kern/ieee1275/ieee1275.c | 1 -
>> grub-core/kern/ieee1275/init.c | 18 +-
>> grub-core/kern/powerpc/ieee1275/ieee1275.c | 139 +++++++
>> .../kern/powerpc/ieee1275/platform_keystore.c | 343 ++++++++++++++++++
>> include/grub/ieee1275/ieee1275.h | 3 +
>> include/grub/powerpc/ieee1275/ieee1275.h | 21 ++
>> .../grub/powerpc/ieee1275/platform_keystore.h | 126 +++++++
>> 9 files changed, 651 insertions(+), 4 deletions(-)
>> create mode 100644 grub-core/kern/powerpc/ieee1275/ieee1275.c
>> create mode 100644 grub-core/kern/powerpc/ieee1275/platform_keystore.c
>> create mode 100644 include/grub/powerpc/ieee1275/platform_keystore.h
>
> [...]
>
>> diff --git a/grub-core/kern/powerpc/ieee1275/platform_keystore.c
>> b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
>> new file mode 100644
>> index 000000000..3af1ea28a
>> --- /dev/null
>> +++ b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
>> @@ -0,0 +1,343 @@
>> +/*
>> + * GRUB -- GRand Unified Bootloader
>> + * Copyright (C) 2024 Free Software Foundation, Inc.
>> + * Copyright (C) 2022, 2023, 2024, 2025 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/mm.h>
>> +#include <grub/types.h>
>> +#include <grub/misc.h>
>> +#include <grub/lockdown.h>
>> +#include <grub/ieee1275/ieee1275.h>
>> +#include <grub/powerpc/ieee1275/ieee1275.h>
>> +#include <grub/powerpc/ieee1275/platform_keystore.h>
>> +
>> +/* PKS consumer type for firmware. */
>> +#define PKS_CONSUMER_FW ((grub_uint8_t) 1)
>> +
>> +/* PKS read object lable for secure boot version. */
>> +#define SB_VERSION_KEY_NAME ((grub_uint8_t *) "SB_VERSION")
>> +#define SB_VERSION_KEY_LEN ((grub_size_t) 10)
>> +
>> +/* PKS read secure boot variable request type for db and dbx. */
>> +#define DB ((grub_uint8_t) 1)
>> +#define DBX ((grub_uint8_t) 2)
>> +
>> +static grub_size_t pks_max_object_size;
>> +grub_uint8_t grub_pks_use_keystore = 0;
>> +
>> +/* Platform Keystore. */
>> +grub_pks_t grub_pks_keystore = { .db = NULL, .dbx = NULL, .db_entries = 0,
>> .dbx_entries = 0 };
>> +
>> +/* Convert the esl data into the ESL. */
>> +static grub_esl_t *
>> +convert_to_esl (const grub_uint8_t *esl_data, const grub_size_t
>> esl_data_size)
>
> This does not convert anything. Just validate sizes and change data
> types. And this function is called once. So, please put its code where
> it belongs....
Will do it.
>
>> +{
>> + grub_esl_t *esl = NULL;
>> +
>> + if (esl_data_size < sizeof (grub_esl_t) || esl_data == NULL)
>> + return esl;
>> +
>> + esl = (grub_esl_t *) esl_data;
>> +
>> + return esl;
>> +}
>> +
>> +/*
>> + * Import the GUID, esd, and its size into the pks sd buffer and
>> + * pks sd entries from the EFI signature list.
>> + */
>> +static grub_err_t
>> +esd_from_esl (const grub_uint8_t *esl_data, grub_size_t esl_size,
>> + const grub_size_t signature_size, const grub_packed_guid_t
>> *guid,
>> + grub_pks_sd_t **pks_sd, grub_size_t *pks_sd_entries)
>> +{
>> + grub_esd_t *esd;
>> + grub_pks_sd_t *signature = *pks_sd;
>> + grub_size_t entries = *pks_sd_entries;
>> + grub_size_t data_size, offset = 0;
>> +
>> + /* Reads the esd from esl. */
>> + while (esl_size > 0)
>> + {
>> + esd = (grub_esd_t *) (esl_data + offset);
>> + data_size = signature_size - sizeof (grub_esd_t);
>> +
>> + signature = grub_realloc (signature, (entries + 1) * sizeof
>> (grub_pks_sd_t));
>> + if (signature == NULL)
>> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory.");
>> +
>> + signature[entries].data = grub_malloc (data_size * sizeof
>> (grub_uint8_t));
>> + if (signature[entries].data == NULL)
>> + {
>> + /*
>> + * Allocated memory will be freed by
>> + * grub_free_platform_keystore.
>> + */
>> + *pks_sd = signature;
>> + *pks_sd_entries = entries + 1;
>> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory.");
>> + }
>> +
>> + grub_memcpy (signature[entries].data, esd->signature_data, data_size);
>> + signature[entries].data_size = data_size;
>> + signature[entries].guid = *guid;
>> + entries++;
>> + esl_size -= signature_size;
>> + offset += signature_size;
>> + }
>> +
>> + *pks_sd = signature;
>> + *pks_sd_entries = entries;
>> +
>> + return GRUB_ERR_NONE;
>> +}
>> +
>> +/* Extract the esd after removing the esl header from esl. */
>> +static grub_err_t
>> +esl_to_esd (const grub_uint8_t *esl_data, grub_size_t *next_esl,
>> + grub_pks_sd_t **pks_sd, grub_size_t *pks_sd_entries)
>> +{
>> + grub_packed_guid_t guid;
>> + grub_esl_t *esl;
>> + grub_size_t offset, esl_size,
>> + signature_size, signature_header_size;
>> +
>> + esl = convert_to_esl (esl_data, *next_esl);
>> + if (esl == NULL)
>> + return grub_error (GRUB_ERR_BUG, "invalid ESL.");
>> +
>> + esl_size = grub_le_to_cpu32 (esl->signature_list_size);
>> + signature_header_size = grub_le_to_cpu32 (esl->signature_header_size);
>> + signature_size = grub_le_to_cpu32 (esl->signature_size);
>> + grub_memcpy (&guid, &esl->signature_type, sizeof (grub_packed_guid_t));
>> +
>> + if (esl_size < sizeof (grub_esl_t) || esl_size > *next_esl)
>> + return grub_error (GRUB_ERR_BUG, "invalid ESL size (%u)\n", esl_size);
>> +
>> + *next_esl = esl_size;
>> + offset = sizeof (grub_esl_t) + signature_header_size;
>> + esl_size = esl_size - offset;
>> +
>> + return esd_from_esl (esl_data + offset, esl_size, signature_size, &guid,
>> + pks_sd, pks_sd_entries);
>> +}
>> +
>> +/*
>> + * Import the EFI signature data and the number of esd from the esl
>> + * into the pks sd buffer and pks sd entries.
>
> Could you be more consistent with using abbreviations, i.e. their case?
> Most of them should be all upper case. And what are esd/ESD" and "sd"?
Will do it.
EFI signature data (ESD) and Signature Data (SD).
>
>
>> + */
>> +static grub_err_t
>> +pks_sd_from_esl (const grub_uint8_t *esl_data, grub_size_t esl_size,
>> + grub_pks_sd_t **pks_sd, grub_size_t *pks_sd_entries)
>> +{
>> + grub_err_t rc;
>> + grub_size_t next_esl = esl_size;
>> +
>> + do
>> + {
>> + rc = esl_to_esd (esl_data, &next_esl, pks_sd, pks_sd_entries);
>> + if (rc != GRUB_ERR_NONE)
>> + break;
>> +
>> + esl_data += next_esl;
>> + esl_size -= next_esl;
>> + next_esl = esl_size;
>> + }
>> + while (esl_size > 0);
>> +
>> + return rc;
>> +}
>> +
>> +/*
>> + * Read the secure boot version from PKS as an object.
>> + * caller must free result.
>> + */
>> +static grub_err_t
>> +read_sbversion_from_pks (grub_uint8_t **out, grub_size_t *outlen,
>> grub_size_t *policy)
>> +{
>> + int rc;
>> +
>> + *out = grub_malloc (pks_max_object_size);
>> + if (*out == NULL)
>> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory.");
>> +
>> + rc = grub_ieee1275_pks_read_object (PKS_CONSUMER_FW, SB_VERSION_KEY_NAME,
>> + SB_VERSION_KEY_LEN, *out,
>> pks_max_object_size,
>> + outlen, policy);
>> + if (rc != 0)
>> + return grub_error (GRUB_ERR_READ_ERROR, "SB version read failed
>> (%d)\n", rc);
>> +
>> + return GRUB_ERR_NONE;
>> +}
>> +
>> +/*
>> + * Reads the secure boot variable from PKS.
>> + * caller must free result.
>
> s/caller/Caller/
Will do it.
>
>> + */
>> +static grub_err_t
>> +read_sbvar_from_pks (const grub_uint8_t sbvarflags, const grub_uint8_t
>> sbvartype,
>> + grub_uint8_t **out, grub_size_t *outlen)
>> +{
>> + int rc;
>> +
>> + *out = grub_malloc (pks_max_object_size);
>> + if (*out == NULL)
>> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory.");
>> +
>> + rc = grub_ieee1275_pks_read_sbvar (sbvarflags, sbvartype, *out,
>> pks_max_object_size, outlen);
>> + if (rc == (int) IEEE1275_CELL_NOT_FOUND)
>
> This cast suggests the rc should be defined as different type…
Will do it.
>
>> + return grub_error (GRUB_ERR_UNKNOWN_COMMAND, "secure boot variable %s
>> not found (%d)",
>> + (sbvartype == DB ? "db" : "dbx"), rc);
>> + else if (rc != 0)
>
> If you look at the code of the grub_ieee1275_pks_read_sbvar() then you
> will see you return "-1" in case of an error. So, I think you should do
> "rc < 0" check here…
Will do it.
>
>> + return grub_error (GRUB_ERR_READ_ERROR, "secure boot variable %s
>> reading (%d)",
>> + (sbvartype == DB ? "db" : "dbx"), rc);
>> +
>> + return GRUB_ERR_NONE;
>> +}
>> +
>> +/* Test the availability of PKS support. */
>> +static grub_err_t
>> +is_support_pks (void)
>> +{
>> + int rc;
>> + grub_ieee1275_cell_t missing = 0;
>> +
>> + rc = grub_ieee1275_test ("pks-max-object-size", &missing);
>> + if (rc != 0 || missing == IEEE1275_CELL_INVALID)
>
> Ditto...
>
>> + grub_error (GRUB_ERR_BAD_FIRMWARE, "firmware doesn't have PKS
>> support.\n");
>> + else
>> + {
>> + rc = grub_ieee1275_pks_max_object_size (&pks_max_object_size);
>> + if (rc != 0)
>
> Ditto... Please fix this problem everywhere…
Will do it.
>
>> + grub_error (GRUB_ERR_BAD_NUMBER, "PKS support is there but it has
>> zero objects.\n");
>> + }
>> +
>> + return GRUB_ERR_NONE;
>> +}
>> +
>> +/*
>> + * Retrieve the secure boot variable from PKS, unpacks it, read the esd
>> + * from ESL, and store the information in the pks sd buffer.
>
> Please be consistent with case...
>
>> + */
>> +static grub_err_t
>> +read_secure_boot_variables (const grub_uint8_t sbvarflags, const
>> grub_uint8_t sbvartype,
>> + grub_pks_sd_t **pks_sd, grub_size_t
>> *pks_sd_entries)
>> +{
>> + grub_err_t rc;
>> + grub_uint8_t *esl_data = NULL;
>> + grub_size_t esl_data_size = 0;
>> +
>> + rc = read_sbvar_from_pks (sbvarflags, sbvartype, &esl_data,
>> &esl_data_size);
>> + if (rc == GRUB_ERR_NONE && esl_data_size != 0)
>> + rc = pks_sd_from_esl ((const grub_uint8_t *) esl_data, esl_data_size,
>> + pks_sd, pks_sd_entries);
>> + grub_free (esl_data);
>> +
>> + return rc;
>> +}
>> +
>> +/*
>> + * Reads secure boot version (SB_VERSION) and it supports following.
>> + * SB_VERSION
>> + * 1 - PKS
>> + * 0 - static key (embeded key)
>
> Same comment as for commit message. We need more info here…
Will do it
>
>> + */
>> +static grub_err_t
>> +get_secure_boot_version (void)
>> +{
>> + grub_err_t rc;
>> + grub_uint8_t *data = NULL;
>> + grub_size_t len = 0, policy = 0;
>> +
>> + rc = read_sbversion_from_pks (&data, &len, &policy);
>> + if (rc == GRUB_ERR_NONE && (len != 1 || (*data >= 2)))
>> + rc = grub_error (GRUB_ERR_BAD_NUMBER, "found unexpected SB version
>> (%d)\n", *data);
>> +
>> + if (rc != GRUB_ERR_NONE)
>> + {
>> + grub_printf ("switch to static key.\n");
>> + if (grub_is_lockdown () == GRUB_LOCKDOWN_ENABLED)
>> + grub_fatal ("secure boot locked down.");
>
> s/secure boot locked down./Secure boot locked down/
>
> However, first of all I think it should be explained here why the
> lockdown cannot be enabled here…
Will do it.
>
>> + }
>> + else
>> + grub_pks_use_keystore = *data;
>
> Why do you assign version number to grub_pks_use_keystore which looks
> like a flag? Probably the name of variable is wrong or its type.
Will change to boolean type.
>
>> + grub_free (data);
>> +
>> + return rc;
>> +}
>> +
>> +/* Free allocated memory. */
>> +void
>> +grub_pks_free_keystore (void)
>> +{
>> + grub_size_t i;
>> +
>> + for (i = 0; i < grub_pks_keystore.db_entries; i++)
>> + grub_free (grub_pks_keystore.db[i].data);
>> +
>> + for (i = 0; i < grub_pks_keystore.dbx_entries; i++)
>> + grub_free (grub_pks_keystore.dbx[i].data);
>> +
>> + grub_free (grub_pks_keystore.db);
>> + grub_free (grub_pks_keystore.dbx);
>> + grub_memset (&grub_pks_keystore, 0, sizeof (grub_pks_t));
>> +}
>> +
>> +/* Initialization of the Platform Keystore. */
>> +grub_err_t
>> +grub_pks_keystore_init (void)
>> +{
>> + grub_err_t rc;
>> +
>> + grub_dprintf ("ieee1275", "trying to load Platform Keystore.\n");
>> +
>> + rc = is_support_pks ();
>> + if (rc != GRUB_ERR_NONE)
>> + {
>> + grub_printf ("switch to static key.\n");
>
> s/grub_printf/grub_dprintf/?
Will do it.
>
>> + return rc;
>> + }
>> +
>> + /* Read SB_VERSION from PKS. */
>> + rc = get_secure_boot_version ();
>> + if (rc != GRUB_ERR_NONE)
>> + return rc;
>> +
>> + if (grub_pks_use_keystore)
>
> This looks like a flag. Should not it be defined as bool?
>
>> + {
>> + grub_memset (&grub_pks_keystore, 0, sizeof (grub_pks_t));
>> + /* Read DB from PKS. */
>> + rc = read_secure_boot_variables (0, DB, &grub_pks_keystore.db,
>> &grub_pks_keystore.db_entries);
>> + if (rc == GRUB_ERR_NONE)
>> + {
>> + /* Read DBX from PKS. */
>> + rc = read_secure_boot_variables (0, DBX, &grub_pks_keystore.dbx,
>> &grub_pks_keystore.dbx_entries);
>> + if (rc == GRUB_ERR_UNKNOWN_COMMAND)
>> + {
>> + grub_dprintf ("ieee1275", "dbx is not found in PKS.\n");
>> + rc = GRUB_ERR_NONE;
>> + }
>> + }
>> +
>> + }
>> +
>> + if (rc != GRUB_ERR_NONE)
>> + grub_pks_free_keystore ();
>> +
>> + return rc;
>> +}
>> diff --git a/include/grub/ieee1275/ieee1275.h
>> b/include/grub/ieee1275/ieee1275.h
>> index c445d0499..3965c5fc6 100644
>> --- a/include/grub/ieee1275/ieee1275.h
>> +++ b/include/grub/ieee1275/ieee1275.h
>> @@ -24,6 +24,9 @@
>> #include <grub/types.h>
>> #include <grub/machine/ieee1275.h>
>>
>> +#define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
>> +#define IEEE1275_CELL_NOT_FOUND ((grub_ieee1275_cell_t) -7)
>> +
>> #define GRUB_IEEE1275_CELL_FALSE ((grub_ieee1275_cell_t) 0)
>> #define GRUB_IEEE1275_CELL_TRUE ((grub_ieee1275_cell_t) -1)
>>
>> diff --git a/include/grub/powerpc/ieee1275/ieee1275.h
>> b/include/grub/powerpc/ieee1275/ieee1275.h
>> index 4eb207018..1772d1f6d 100644
>> --- a/include/grub/powerpc/ieee1275/ieee1275.h
>> +++ b/include/grub/powerpc/ieee1275/ieee1275.h
>> @@ -28,4 +28,25 @@ typedef grub_uint32_t grub_ieee1275_cell_t;
>> #define PRIxGRUB_IEEE1275_CELL_T PRIxGRUB_UINT32_T
>> #define PRIuGRUB_IEEE1275_CELL_T PRIuGRUB_UINT32_T
>>
>> +#ifdef __powerpc__
>> +
>> +extern int
>> +EXPORT_FUNC (grub_ieee1275_test) (const char *name,
>> + grub_ieee1275_cell_t *missing);
>> +
>> +extern int
>> +grub_ieee1275_pks_max_object_size (grub_size_t *result);
>> +
>> +extern int
>> +grub_ieee1275_pks_read_object (grub_uint8_t consumer, grub_uint8_t *label,
>> + grub_size_t label_len, grub_uint8_t *buffer,
>> + grub_size_t buffer_len, grub_size_t
>> *data_len,
>> + grub_uint32_t *policies);
>> +
>> +extern int
>> +grub_ieee1275_pks_read_sbvar (grub_uint8_t sbvarflags, grub_uint8_t
>> sbvartype,
>> + grub_uint8_t *buffer, grub_size_t buffer_len,
>> + grub_size_t *data_len);
>
> It seems to me at least some of these functions should not be exported.
> If they are used in following patches then they should be exported when
> a given patch adds calls to these functions.
Will do it.
>
>> +#endif
>> +
>> #endif /* ! GRUB_IEEE1275_MACHINE_HEADER */
>> diff --git a/include/grub/powerpc/ieee1275/platform_keystore.h
>> b/include/grub/powerpc/ieee1275/platform_keystore.h
>> new file mode 100644
>> index 000000000..d48c60200
>> --- /dev/null
>> +++ b/include/grub/powerpc/ieee1275/platform_keystore.h
>> @@ -0,0 +1,126 @@
>> +/*
>> + * Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved. This
>> + * program and the accompanying materials are licensed and made available
>> + * under the terms and conditions of the 2-Clause BSD License which
>> + * accompanies this distribution.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions are
>> met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright notice,
>> + * this list of conditions and the following disclaimer.
>> + *
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + * notice, this list of conditions and the following disclaimer in the
>> + * documentation and/or other materials provided with the distribution.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
>> IS"
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>> THE
>> + * POSSIBILITY OF SUCH DAMAGE.
>> + *
>> + * https://github.com/tianocore/edk2-staging (edk2-staging repo of
>> tianocore),
>> + * the ImageAuthentication.h file under it, and here's the copyright and
>> license.
>> + *
>> + * MdePkg/Include/Guid/ImageAuthentication.h
>> + *
>> + * Copyright 2022, 2023, 2024, 2025 IBM Corp.
>> + */
>> +
>> +#ifndef __PLATFORM_KEYSTORE_H__
>> +#define __PLATFORM_KEYSTORE_H__
>> +
>> +#include <grub/symbol.h>
>> +#include <grub/mm.h>
>> +#include <grub/types.h>
>> +
>> +#if __GNUC__ >= 9
>> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>> +#endif
>
> This probably should not be in the header file…
Will take care.
>
>> +#define GRUB_MAX_HASH_SIZE 64
>> +
>> +/* Secure Boot Mode. */
>> +#define SB_DISABLED 0
>> +#define SB_ENFORCED 2
>> +
>> +/*
>> + * It is derived from EFI_SIGNATURE_DATA
>> + *
>> https://github.com/tianocore/edk2-staging/blob/master/MdePkg/Include/Guid/ImageAuthentication.h
>> + *
>> + * The structure of an EFI signature database (ESD).*/
>
> Aha... Here it is... So, you should use correct ESD abbreviation
> everywhere...
>
> And missing space after "." here and below...
Will do it.
>
>> +struct grub_esd
>> +{
>> + /*
>> + * An identifier which identifies the agent which added
>> + * the signature to the list.
>> + */
>> + grub_packed_guid_t signature_owner;
>> + /* The format of the signature is defined by the SignatureType.*/
>> + grub_uint8_t signature_data[];
>> +} GRUB_PACKED;
>> +
>
> Please drop this empty line...
>
>> +typedef struct grub_esd grub_esd_t;
>> +
>> +/*
>> + * It is derived from EFI_SIGNATURE_LIST
>> + *
>> https://github.com/tianocore/edk2-staging/blob/master/MdePkg/Include/Guid/ImageAuthentication.h
>> + *
>> + * The structure of an EFI signature list (ESL).*/
>> +struct grub_esl
>> +{
>> + /* Type of the signature. GUID signature types are defined in below.*/
>> + grub_packed_guid_t signature_type;
>> + /* Total size of the signature list, including this header.*/
>> + grub_uint32_t signature_list_size;
>> + /*
>> + * Size of the signature header which precedes
>> + * the array of signatures.
>> + */
>> + grub_uint32_t signature_header_size;
>> + /* Size of each signature.*/
>> + grub_uint32_t signature_size;
>> +} GRUB_PACKED;
>> +
>
> Ditto...
>
>> +typedef struct grub_esl grub_esl_t;
>> +
>> +/* The structure of a PKS signature data.*/
>
> Ha! sd == signature data. So, if you really use sd abbreviation in the
> comments please define it here, e.g. Signature Data (SD), and use it
> consistently.
Sure will do it.
>
>> +struct grub_pks_sd
>> +{
>> + grub_packed_guid_t guid; /* Signature type. */
>> + grub_uint8_t *data; /* Signature data. */
>> + grub_size_t data_size; /* Size of signature data. */
>> +} GRUB_PACKED;
>> +
>
> Redundant empty line...
>
>> +typedef struct grub_pks_sd grub_pks_sd_t;
>> +
>> +/* The structure of a PKS.*/
>> +struct grub_pks
>> +{
>> + grub_pks_sd_t *db; /* Signature database. */
>> + grub_pks_sd_t *dbx; /* Forbidden signature database. */
>> + grub_size_t db_entries; /* Size of signature database. */
>> + grub_size_t dbx_entries; /* Size of forbidden signature database. */
>> +} GRUB_PACKED;
>> +
>
> Ditto...
>
>> +typedef struct grub_pks grub_pks_t;
>> +
>> +/* Initialization of the Platform Keystore. */
>> +extern grub_err_t
>> +grub_pks_keystore_init (void);
>> +
>> +/* Free allocated memory. */
>> +extern void
>> +EXPORT_FUNC (grub_pks_free_keystore) (void);
>> +
>> +extern grub_uint8_t EXPORT_VAR (grub_pks_use_keystore);
>> +extern grub_pks_t EXPORT_VAR (grub_pks_keystore);
>
> Really, do these stuff need to be exported?
These are used in following patches (appended signature module). So, I exported
it.
Thanks,
Sudhakar
>
> Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
