On 6/16/23 13:34, Stefan Herbrechtsmeier wrote:

From: Malte Schmidt malte.schmidt@weidmueller.com

Thanks for considering which parameters may be constants.

nits:

The Urban Dictionary defines 'constify' as:

"To constantly do something, like constantly watching anime all day."

%s/constify function parameters/make function parameters const/

Use const keyword for function parameters where appropriate.

Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

tools/mkeficapsule.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index 52be1f122e..b8db00b16b 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -88,8 +88,8 @@ static void print_usage(void)

• are filled in by create_auth_data().

*/ struct auth_context {

• char *key_file;
• char *cert_file;

• const char *key_file;
• const char *cert_file; uint8_t *image_data; size_t image_size; struct efi_firmware_image_authentication auth;

@@ -112,7 +112,7 @@ static int dump_sig;

• • 0 - on success
• • -1 - on failure

*/ -static int read_bin_file(char *bin, uint8_t **data, off_t *bin_size) +static int read_bin_file(const char *bin, uint8_t **data, off_t *bin_size) { FILE *g; struct stat bin_stat; @@ -170,7 +170,8 @@ err:

• • 0 - on success
• • -1 - on failure

*/ -static int write_capsule_file(FILE *f, void *data, size_t size, const char *msg) +static int write_capsule_file(FILE *f, const void *data, size_t size,

Why should size not be constant?

The name of the parameter 'size' does not match the function documentation which has 'bin_size'. Please, change either of both.

Parameter 'f' does not match the documentation which has 'bin'.

For each function that you touch, please, ensure that the function parameters are correctly documented.

• 	      const char *msg)
  

  { size_t size_written;

@@ -343,7 +344,8 @@ static int create_auth_data(struct auth_context *ctx)

• • 0 - on success
• • -1 - on failure

*/ -static int dump_signature(const char *path, uint8_t *signature, size_t sig_size) +static int dump_signature(const char *path, const uint8_t *signature,

• 	  size_t sig_size)
  

Why should sig_size not be constant?

{ char *sig_path; FILE *f; @@ -402,10 +404,12 @@ static void free_sig_data(struct auth_context *ctx)

• • 0 - on success
• • -1 - on failure

*/ -static int create_fwbin(char *path, char *bin, efi_guid_t *guid,

• 	unsigned long index, unsigned long instance,
  

• 	struct fmp_payload_header_params *fmp_ph_params,
  

• 	uint64_t mcount, char *privkey_file, char *cert_file,
  

+static int create_fwbin(const char *path, const char *bin,

• 	const efi_guid_t *guid, unsigned long index,
  

• 	unsigned long instance,
  

• 	const struct fmp_payload_header_params *fmp_ph_params,
  

• 	uint64_t mcount,
  

• 	const char *privkey_file, const char *cert_file,
  uint16_t oemflags)
  

Why shouldn't instance, mcount, oemflags be constant?

{ struct efi_capsule_header header; @@ -604,7 +608,8 @@ void convert_uuid_to_guid(unsigned char *buf) buf[7] = c; }

-static int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept) +static int create_empty_capsule(const char *path, const efi_guid_t *guid,

• 		bool fw_accept)
  

Why should fw_accept not be constant?

Please, make the use of 'const' a bit more consistent.

Best regards

Heinrich

{ struct efi_capsule_header header = { 0 }; FILE *f = NULL; @@ -666,7 +671,7 @@ int main(int argc, char **argv) unsigned long index, instance; uint64_t mcount; unsigned long oemflags;

• char *privkey_file, *cert_file;

• const char *privkey_file, *cert_file; int c, idx; struct fmp_payload_header_params fmp_ph_params = { 0 };

On Fri, 16 Jun 2023 at 17:56, Sughosh Ganu sughosh.ganu@linaro.org wrote:

hi Stefan,

On Fri, 16 Jun 2023 at 17:04, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:

...

From: Malte Schmidt malte.schmidt@weidmueller.com

The UEFI [1] specification allows multiple payloads inside the capsule body. Add support for this. The command line arguments are kept backwards-compatible.

[1] https://uefi.org/specs/UEFI/2.10/index.html

I am trying to upstream support for specifying the capsule parameters for multiple payloads through a config file [1]. This is on similar lines to the support in the Edk2 GenerateCapule tool where multiple payloads can be specified through a json file. I think you can base your changes on my series.

Btw, with the support being added for getting the capsule parameters through a config file, I believe your changes would be pretty much simplified. Instead of passing all those parameters through the command line, they can instead be read from the config file and used to generate a single capsule file consisting of multiple payloads. That would be a much simpler implementation.

-sughosh

-sughosh

[1] - https://lore.kernel.org/u-boot/20230613103806.812065-1-sughosh.ganu@linaro.o...

...

Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

tools/eficapsule.h | 5 - tools/mkeficapsule.c | 636 ++++++++++++++++++++++++++++++++----------- 2 files changed, 475 insertions(+), 166 deletions(-)

diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..001af3217c 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -138,9 +138,4 @@ struct fmp_payload_header { uint32_t lowest_supported_version; };

-struct fmp_payload_header_params {

•   bool have_header;
  

•   uint32_t fw_version;
  

-};

#endif /* _EFI_CAPSULE_H */ diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index b8db00b16b..1a4de0f092 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule"; efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;

-static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR"; +static const char *opts_short = "g:i:b:I:v:p:c:m:o:dhAR";

enum { CAPSULE_NORMAL_BLOB = 0, @@ -40,6 +40,7 @@ enum { static struct option options[] = { {"guid", required_argument, NULL, 'g'}, {"index", required_argument, NULL, 'i'},

•   {"image_blob", required_argument, NULL, 'b'},
    {"instance", required_argument, NULL, 'I'},
    {"fw-version", required_argument, NULL, 'v'},
    {"private-key", required_argument, NULL, 'p'},
  

@@ -55,21 +56,22 @@ static struct option options[] = {

static void print_usage(void) {

•   fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
  

•   fprintf(stderr, "Usage: %s [options] [<image blob>] <output file>\n"
            "Options:\n"
  
  

•           "\t-g, --guid <guid string>    guid for image blob type\n"
  

•           "\t-i, --index <index>         update image index\n"
  

•           "\t-I, --instance <instance>   update hardware instance\n"
  

•           "\t-v, --fw-version <version>  firmware version\n"
  

•           "\t-p, --private-key <privkey file>  private key file\n"
  

•           "\t-c, --certificate <cert file>     signer's certificate file\n"
  

•           "\t-m, --monotonic-count <count>     monotonic count\n"
  

•           "\t-d, --dump_sig              dump signature (*.p7)\n"
  

•           "\t-A, --fw-accept  firmware accept capsule, requires GUID, no image blob\n"
  

•           "\t-R, --fw-revert  firmware revert capsule, takes no GUID, no image blob\n"
  

•           "\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n"
  

•           "\t-h, --help                  print a help message\n",
  

•           "\t-g, --guid <guid list>           comma-separated list of guids for image blob types\n"
  

•           "\t-i, --index <index list>         comma-separated list of update image indices\n"
  

•           "\t-b, --image_blob <blob list>     comma-separated list of image blobs\n"
  

•           "\t-I, --instance <instance list>   comma-separated list of update hardware instances\n"
  

•           "\t-v, --fw-version <version list>  comma-separated list of firmware versions\n"
  

•           "\t-p, --private-key <privkey file>              private key file\n"
  

•           "\t-c, --certificate <cert file>                 signer's certificate file\n"
  

•           "\t-m, --monotonic-count <monotonic-count list>  comma-separated list of monotonic counts\n"
  

•           "\t-d, --dump_sig                                dump signature (*.p7)\n"
  

•           "\t-A, --fw-accept   firmware accept capsule, requires GUID, no image blob\n"
  

•           "\t-R, --fw-revert   firmware revert capsule, takes no GUID, no image blob\n"
  

•           "\t-o, --capoemflag  capsule OEM Flag, an integer between 0x0000 and 0xffff\n"
  

•           "\t-h, --help        print a help message\n",
            tool_name);
  

}

@@ -336,16 +338,18 @@ static int create_auth_data(struct auth_context *ctx)

• @path: Path to a capsule file
• @signature: Signature data
• @sig_size: Size of signature data

• • @index: The payload index the signature belongs to
  • Signature data pointed to by @signature will be saved into

• • a file whose file name is @path with ".p7" suffix.

• • a file whose file name is @path with "_<index>.p7" suffix.
• • If index is negative the suffix is ".p7" (for backwards compatibility).
  • Return:
  • • 0 - on success
  • • -1 - on failure
  */

static int dump_signature(const char *path, const uint8_t *signature,

•                     size_t sig_size)
  

•                     size_t sig_size, int index)
  

{ char *sig_path; FILE *f; @@ -356,7 +360,11 @@ static int dump_signature(const char *path, const uint8_t *signature, if (!sig_path) return ret;

•   sprintf(sig_path, "%s.p7", path);
  

•   if (index < 0)
  

•           sprintf(sig_path, "%s.p7", path);
  

•   else
  

•           sprintf(sig_path, "%s_%d.p7", path, index);
  

•   f = fopen(sig_path, "w");
    if (!f)
            goto err;
  

@@ -386,14 +394,15 @@ static void free_sig_data(struct auth_context *ctx) /**

• create_fwbin - create an uefi capsule file
• @path: Path to a created capsule file

• • @bin: Path to a firmware binary to encapsulate
• • @guid: GUID of related FMP driver
• • @index: Index number in capsule

• • @bins: Paths to firmware binaries to encapsulate, an array
• • @guids: GUIDs of related FMP drivers, an array
• • @indices: Index numbers in capsule, an array
  • @instance: Instance number in capsule
  • @mcount: Monotonic count in authentication information
• • @size: Size of the arrays
  • @private_file: Path to a private key file
  • @cert_file: Path to a certificate file

• • @oemflags: Capsule OEM Flags, bits 0-15

• • @oemflags: Capsule OEM Flags, bits 0-15
  • This function actually does the job of creating an uefi capsule file.
  • All the arguments must be supplied.

@@ -404,78 +413,87 @@ static void free_sig_data(struct auth_context *ctx)

• • 0 - on success
• • -1 - on failure

*/ -static int create_fwbin(const char *path, const char *bin,

•                   const efi_guid_t *guid, unsigned long index,
  

•                   unsigned long instance,
  

•                   const struct fmp_payload_header_params *fmp_ph_params,
  

•                   uint64_t mcount,
  

•                   const char *privkey_file, const char *cert_file,
  

•                   uint16_t oemflags)
  

+static int create_fwbin(const char *path, const char **bins,

•                   const efi_guid_t *guids, const unsigned long *indices,
  

•                   const unsigned long *instances,
  

•                   const unsigned long *fw_versions, const unsigned long *mcounts,
  

•                   int size, const char *privkey_file,
  

•                   const char *cert_file, uint16_t oemflags)
  

{ struct efi_capsule_header header; struct efi_firmware_management_capsule_header capsule;

•   struct efi_firmware_management_capsule_image_header image;
  

•   struct auth_context auth_context;
  

•   struct efi_firmware_management_capsule_image_header images[size];
  

•   struct auth_context auth_contexts[size];
    FILE *f;
  

•   uint8_t *data, *new_data, *buf;
  

•   off_t bin_size;
  

•   uint64_t offset;
  

•   uint8_t *data_list[size], *new_data_list[size], *buf_list[size];
  

•   off_t bin_sizes[size];
  

•   uint64_t offsets[size];
    int ret;
  

•   struct fmp_payload_header payload_header;
  

•   struct fmp_payload_header payload_headers[size];
  
  

#ifdef DEBUG fprintf(stderr, "For output: %s\n", path);

•   fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid);
  

•   fprintf(stderr, "\tindex: %lu\n\tinstance: %lu\n", index, instance);
  

•   for (int i = 0; i < size; i++) {
  

•           fprintf(stderr, "\tpayload no: %d\n", i);
  

•           fprintf(stderr, "\t\tbin: %s\n\t\ttype: %pUl\n", bins[i], guids[i]);
  

•           fprintf(stderr, "\t\tindex: %lu\n\t\tinstance: %lu\n", indices[i], instances[i]);
  

•   }
  

#endif

•   auth_context.sig_size = 0;
    f = NULL;
  

•   data = NULL;
  

•   new_data = NULL;
    ret = -1;
  
  

•   /*
  

•    * read a firmware binary
  

•    */
  

•   if (read_bin_file(bin, &data, &bin_size))
  

•           goto err;
  

•   for (int i = 0; i < size; i++) {
  

•           auth_contexts[i].sig_size = 0;
  

•           data_list[i] = NULL;
  

•           new_data_list[i] = NULL;
  

•   }
  
  

•   buf = data;
  

•   for (int i = 0; i < size; i++) {
  

•           int dump_index = (size == 1) ? -1 : i;
  
  

•   /* insert fmp payload header right before the payload */
  

•   if (fmp_ph_params->have_header) {
  

•           new_data = malloc(bin_size + sizeof(payload_header));
  

•           if (!new_data)
  

•           /*
  

•            * read a firmware binary
  

•            */
  

•           if (read_bin_file(bins[i], &data_list[i], &bin_sizes[i]))
                    goto err;
  
  

•           payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE;
  

•           payload_header.header_size = sizeof(payload_header);
  

•           payload_header.fw_version = fmp_ph_params->fw_version;
  

•           payload_header.lowest_supported_version = 0; /* not used */
  

•           memcpy(new_data, &payload_header, sizeof(payload_header));
  

•           memcpy(new_data + sizeof(payload_header), data, bin_size);
  

•           buf = new_data;
  

•           bin_size += sizeof(payload_header);
  

•   }
  

•   /* first, calculate signature to determine its size */
  

•   if (privkey_file && cert_file) {
  

•           auth_context.key_file = privkey_file;
  

•           auth_context.cert_file = cert_file;
  

•           auth_context.auth.monotonic_count = mcount;
  

•           auth_context.image_data = buf;
  

•           auth_context.image_size = bin_size;
  

•           if (create_auth_data(&auth_context)) {
  

•                   fprintf(stderr, "Signing firmware image failed\n");
  

•                   goto err;
  

•           buf_list[i] = data_list[i];
  

•           /* insert fmp payload header right before the payload */
  

•           if (fw_versions) {
  

•                   new_data_list[i] = malloc(bin_sizes[i] + sizeof(payload_headers[i]));
  

•                   if (!new_data_list[i])
  

•                           goto err;
  

•                   payload_headers[i].signature = FMP_PAYLOAD_HDR_SIGNATURE;
  

•                   payload_headers[i].header_size = sizeof(payload_headers[i]);
  

•                   payload_headers[i].fw_version = fw_versions[i];
  

•                   payload_headers[i].lowest_supported_version = 0; /* not used */
  

•                   memcpy(new_data_list[i], (payload_headers + i), sizeof(payload_headers[i]));
  

•                   memcpy(new_data_list[i] + sizeof(payload_headers[i]), data_list[i],
  

•                          bin_sizes[i]);
  

•                   buf_list[i] = new_data_list[i];
  

•                   bin_sizes[i] += sizeof(payload_headers[i]);
            }
  
  

•           if (dump_sig &&
  

•               dump_signature(path, auth_context.sig_data,
  

•                              auth_context.sig_size)) {
  

•                   fprintf(stderr, "Creating signature file failed\n");
  

•                   goto err;
  

•           /* calculate signature to determine its size */
  

•           if (privkey_file && cert_file) {
  

•                   auth_contexts[i].key_file = privkey_file;
  

•                   auth_contexts[i].cert_file = cert_file;
  

•                   auth_contexts[i].auth.monotonic_count = mcounts[i];
  

•                   auth_contexts[i].image_data = buf_list[i];
  

•                   auth_contexts[i].image_size = bin_sizes[i];
  

•                   if (create_auth_data(&auth_contexts[i])) {
  

•                           fprintf(stderr, "Signing firmware image failed\n");
  

•                           goto err;
  

•                   }
  

•                   if (dump_sig &&
  

•                       dump_signature(path, auth_contexts[i].sig_data,
  

•                                      auth_contexts[i].sig_size, dump_index)) {
  

•                           fprintf(stderr, "Creating signature file failed\n");
  

•                           goto err;
  

•                   }
            }
    }
  
  

@@ -498,81 +516,87 @@ static int create_fwbin(const char *path, const char *bin, if (oemflags) header.flags |= oemflags; header.capsule_image_size = sizeof(header)

•                                   + sizeof(capsule) + sizeof(uint64_t)
  

•                                   + sizeof(image)
  

•                                   + bin_size;
  

•   if (auth_context.sig_size)
  

•           header.capsule_image_size += sizeof(auth_context.auth)
  

•                           + auth_context.sig_size;
  

•                                   + sizeof(capsule)
  

•                                   + size * sizeof(uint64_t); /* size of item_offset_list */
  

•   for (int i = 0; i < size; i++) {
  

•           offsets[i] = header.capsule_image_size - sizeof(header);
  

•           header.capsule_image_size += sizeof(images[i])
  

•                                   + bin_sizes[i];
  

•           if (auth_contexts[i].sig_size)
  

•                   header.capsule_image_size += sizeof(auth_contexts[i].auth)
  

•                                   + auth_contexts[i].sig_size;
  

•   }
    if (write_capsule_file(f, &header, sizeof(header),
                           "Capsule header"))
            goto err;
  
    /*
     * firmware capsule header
  

•    * This capsule has only one firmware capsule image.
     */
    capsule.version = 0x00000001;
    capsule.embedded_driver_count = 0;
  

•   capsule.payload_item_count = 1;
  

•   capsule.payload_item_count = size;
    if (write_capsule_file(f, &capsule, sizeof(capsule),
                           "Firmware capsule header"))
            goto err;
  
  

•   offset = sizeof(capsule) + sizeof(uint64_t);
  

•   if (write_capsule_file(f, &offset, sizeof(offset),
  

•                          "Offset to capsule image"))
  

•   if (write_capsule_file(f, &offsets, size * sizeof(uint64_t),
  

•                          "Offsets to capsule images"))
            goto err;
  
  

•   /*
  

•    * firmware capsule image header
  

•    */
  

•   image.version = 0x00000003;
  

•   memcpy(&image.update_image_type_id, guid, sizeof(*guid));
  

•   image.update_image_index = index;
  

•   image.reserved[0] = 0;
  

•   image.reserved[1] = 0;
  

•   image.reserved[2] = 0;
  

•   image.update_image_size = bin_size;
  

•   if (auth_context.sig_size)
  

•           image.update_image_size += sizeof(auth_context.auth)
  

•                           + auth_context.sig_size;
  

•   image.update_vendor_code_size = 0; /* none */
  

•   image.update_hardware_instance = instance;
  

•   image.image_capsule_support = 0;
  

•   if (auth_context.sig_size)
  

•           image.image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION;
  

•   if (write_capsule_file(f, &image, sizeof(image),
  

•                          "Firmware capsule image header"))
  

•           goto err;
  

•   /*
  

•    * signature
  

•    */
  

•   if (auth_context.sig_size) {
  

•           if (write_capsule_file(f, &auth_context.auth,
  

•                                  sizeof(auth_context.auth),
  

•                                  "Authentication header"))
  

•   for (int i = 0; i < size; i++) {
  

•           /*
  

•            * firmware capsule image header
  

•            */
  

•           images[i].version = 0x00000003;
  

•           memcpy(&images[i].update_image_type_id, &guids[i], sizeof(guids[i]));
  

•           images[i].update_image_index = indices[i];
  

•           images[i].reserved[0] = 0;
  

•           images[i].reserved[1] = 0;
  

•           images[i].reserved[2] = 0;
  

•           images[i].update_image_size = bin_sizes[i];
  

•           if (auth_contexts[i].sig_size)
  

•                   images[i].update_image_size += sizeof(auth_contexts[i].auth)
  

•                                   + auth_contexts[i].sig_size;
  

•           images[i].update_vendor_code_size = 0; /* none */
  

•           images[i].update_hardware_instance = instances[i];
  

•           images[i].image_capsule_support = 0;
  

•           if (auth_contexts[i].sig_size)
  

•                   images[i].image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION;
  

•           if (write_capsule_file(f, &images[i], sizeof(images[i]),
  

•                                  "Firmware capsule image header"))
                    goto err;
  
  

•           if (write_capsule_file(f, auth_context.sig_data,
  

•                                  auth_context.sig_size, "Signature"))
  

•           /*
  

•            * signature
  

•            */
  

•           if (auth_contexts[i].sig_size) {
  

•                   if (write_capsule_file(f, &auth_contexts[i].auth,
  

•                                          sizeof(auth_contexts[i].auth),
  

•                                          "Authentication header"))
  

•                           goto err;
  

•                   if (write_capsule_file(f, auth_contexts[i].sig_data,
  

•                                          auth_contexts[i].sig_size, "Signature"))
  

•                           goto err;
  

•           }
  

•           /*
  

•            * firmware binary
  

•            */
  

•           if (write_capsule_file(f, buf_list[i], bin_sizes[i], "Firmware binary"))
                    goto err;
    }
  
  

•   /*
  

•    * firmware binary
  

•    */
  

•   if (write_capsule_file(f, buf, bin_size, "Firmware binary"))
  

•           goto err;
  

•   ret = 0;
  

err: if (f) fclose(f);

•   free_sig_data(&auth_context);
  

•   free(data);
  

•   free(new_data);
  

•   for (int i = 0; i < size; i++) {
  

•           free_sig_data(&auth_contexts[i]);
  

•           free(data_list[i]);
  

•           free(new_data_list[i]);
  

•   }
  
    return ret;
  

} @@ -652,6 +676,228 @@ err: return ret; }

+/**

• • count_items - count number of items in list
• • @list: Pointer to a string
• • @separator: Separator used to separate list items
• • Count the number of items in a list. The list items
• • are separated by a separator character inside the string.
• • Trailing white spaces are not allowed except if it is the separator.
• • Return:
• • The item count.
• */

+int count_items(const char *list, char separator) +{

•   const char *c;
  

•   int count = 0;
  

•   if (!*list)
  

•           return 0;
  

•   for (c = list; *c; c++) {
  

•           if (*c == separator)
  

•                   count++;
  

•   }
  

•   /* correct count if no trailing separator present */
  

•   if (*(c - 1) != separator)
  

•           count++;
  

•   return count;
  

+}

+/**

• • update_itemcount - update item count
• • @count: The count to be updated
• • @list: The item list
• • @separator: List separator
• • Initialize the count if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with the list count.
• • Return:
• • • 0 - on success
• • • -1 - if a check fails
• */

+int update_itemcount(int *count, const char *list, char separator) +{

•   int current_count = count_items(list, separator);
  

•   if (*count < 0)
  

•           *count = current_count;
  

•   if (*count == 0 ||
  

•       *count != current_count)
  

•           return -1;
  

•   return 0;
  

+}

+/**

• • split_list - split list into elements
• • @elements: Pointer to string array
• • @size: The array size
• • @list: The item list
• • @separator: List separator
• • Split a comma-separated list into its elements.
• • Return:
• • • 0 - on success
• • • -1 - on failure
• */

+int split_list(char **elements, int size, char *list, char separator) +{

•   const char separator_str[] = {separator, '\0'};
  

•   char *end;
  

•   for (int i = 0; i < size; i++) {
  

•           elements[i] = strsep(&list, separator_str);
  

•           if (!elements[i])
  

•                   return -1;
  

•   }
  

•   end = strsep(&list, separator_str);  /* NULL or empty string expected */
  

•   if (end && *end)
  

•           return -1;
  

•   return 0;
  

+}

+/**

• • alloc_array - allocate memory for array
• • @count: The number of elements
• • @obj_size: The size of a single element
• • @name: The name of the array
• • This is a wrapper for malloc which prints an error
• • message on failure.
• • Return:
• • • Pointer to the allocated memory on success
• • • NULL on failure
• */

+void *alloc_array(unsigned int count, size_t obj_size, const char *name) +{

•   void *array;
  

•   array = malloc(count * obj_size);
  

•   if (!array)
  

•           fprintf(stderr, "Could not allocate memory for %s\n", name);
  

•   return array;
  

+}

+/**

• • init_guids - populate guid array
• • @elements: String array of elements to be converted
• • @size: The array size
• • @name: The name of the array
• • Allocate and populate an array of guid structs. The list contains the UUIDs
• • to convert and store in the array. Upon failure an error message is
• • printed.
• • Return:
• • • The initialized GUID array on success
• • • NULL on failure
• */

+efi_guid_t *init_guids(const char **elements, unsigned int size,

•                  const char *name)
  

+{

•   efi_guid_t *guids;
  

•   guids = alloc_array(size, sizeof(efi_guid_t), name);
  

•   if (!guids)
  

•           return NULL;
  

•   for (int i = 0; i < size; i++) {
  

•           if (uuid_parse(elements[i], (unsigned char *)(guids + i))) {
  

•                   fprintf(stderr, "Wrong %s format\n", name);
  

•                   free(guids);
  

•                   return NULL;
  

•           }
  

•           convert_uuid_to_guid((unsigned char *)(guids + i));
  

•   }
  

•   return guids;
  

+}

+/**

• • init_uls - populate unsigned long array
• • @elements: String array of elements to be converted
• • @size: The array size
• • @name: The name of the array
• • Allocate and populate an array of unsgined longs. Upon failure an
• • error message is printed.
• • Return:
• • • The initialized array on success
• • • NULL on failure
• */

+unsigned long *init_uls(const char **elements, unsigned int size,

•                   const char *name)
  

+{

•   unsigned long *array;
  

•   array = alloc_array(size, sizeof(unsigned long), name);
  

•   if (!array)
  

•           return NULL;
  

•   for (int i = 0; i < size; i++)
  

•           array[i] = strtoul(elements[i], NULL, 0);
  

•   return array;
  

+}

+/**

• • init_list - parse list and allocate elements
• • @listcount: The list count to be checked and updated
• • @list: The list to be parsed
• • @separator: The list separator
• • @name: The name of the list
• • @multiple_times: List encountered multiple times
• • Routine for command line argument lists.
• • Parse the string list and count the list elements.
• • Initialize the listcount if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with the list count.
• • Allocate the string array and populate it with the list elements.
• • The array should be freed in the calling function.
• • Upon failure an error message is printed and the program exits.
• • Return:
• • • The initialized array on success
• • • NULL on failure
• */

+char **init_list(int *listcount, char *list, char separator,

•            bool multiple_times, char *name)
  

+{

•   char **elements;
  

•   if (multiple_times) {
  

•           fprintf(stderr, "%s specified multiple times\n", name);
  

•           return NULL;
  

•   }
  

•   if (update_itemcount(listcount, list, separator)) {
  

•           fprintf(stderr, "List count not consistent with previous or list not provided\n");
  

•           return NULL;
  

•   }
  

•   elements = alloc_array(*listcount, sizeof(char *), name);
  

•   if (!elements)
  

•           return NULL;
  

•   if (split_list(elements, *listcount, list, separator)) {
  

•           fprintf(stderr, "Could not parse %s list\n", name);
  

•           free(elements);
  

•           return NULL;
  

•   }
  

•   return elements;
  

+}

/**

• main - main entry function of mkeficapsule
• @argc: Number of arguments

@@ -666,24 +912,27 @@ err: */ int main(int argc, char **argv) {

•   efi_guid_t *guid;
  

•   unsigned char uuid_buf[16];
  

•   unsigned long index, instance;
  

•   uint64_t mcount;
  

•   const char separator = ',';
  

•   const efi_guid_t *guids; /* an array */
  

•   const unsigned long *indices, *instances, *mcounts, *fw_versions; /* arrays */
    unsigned long oemflags;
  

•   const char **blob_paths, **elements;  /* string arrays */
    const char *privkey_file, *cert_file;
  

•   int c, idx;
  

•   struct fmp_payload_header_params fmp_ph_params = { 0 };
  

•   int listcount, c, idx;
  
  

•   guid = NULL;
  

•   index = 0;
  

•   instance = 0;
  

•   mcount = 0;
  

•   guids = NULL;
  

•   indices = NULL;
  

•   instances = NULL;
  

•   mcounts = NULL;
  

•   oemflags = 0;
  

•   blob_paths = NULL;
    privkey_file = NULL;
    cert_file = NULL;
  

•   elements = NULL;
  

•   listcount = -1;
  

•   fw_versions = NULL;
    dump_sig = 0;
    capsule_type = CAPSULE_NORMAL_BLOB;
  

•   oemflags = 0;
    for (;;) {
            c = getopt_long(argc, argv, opts_short, options, &idx);
            if (c == -1)
  

@@ -691,27 +940,62 @@ int main(int argc, char **argv)

            switch (c) {
            case 'g':

•                   if (guid) {
  

•                           fprintf(stderr,
  

•                                   "Image type already specified\n");
  

•                   elements = (const char **)init_list(&listcount, optarg, separator, !!guids,
  

•                                                       "GUID");
  

•                   if (!elements)
                            exit(EXIT_FAILURE);
  

•                   }
  

•                   if (uuid_parse(optarg, uuid_buf)) {
  

•                           fprintf(stderr, "Wrong guid format\n");
  

•                   guids = init_guids(elements, listcount, "GUID");
  

•                   if (!guids)
                            exit(EXIT_FAILURE);
  

•                   }
  

•                   convert_uuid_to_guid(uuid_buf);
  

•                   guid = (efi_guid_t *)uuid_buf;
  

•                   free(elements);
  

•                   elements = NULL;
                    break;
            case 'i':
  

•                   index = strtoul(optarg, NULL, 0);
  

•                   elements = (const char **)init_list(&listcount, optarg, separator,
  

•                                                       !!indices, "index");
  

•                   if (!elements)
  

•                           exit(EXIT_FAILURE);
  

•                   indices = init_uls(elements, listcount, "index");
  

•                   if (!indices)
  

•                           exit(EXIT_FAILURE);
  

•                   free(elements);
  

•                   elements = NULL;
  

•                   break;
  

•           case 'b':
  

•                   blob_paths = (const char **)init_list(&listcount, optarg, separator,
  

•                                                         !!blob_paths, "blob path");
  

•                   if (!blob_paths)
  

•                           exit(EXIT_FAILURE);
                    break;
            case 'I':
  

•                   instance = strtoul(optarg, NULL, 0);
  

•                   elements = (const char **)init_list(&listcount, optarg, separator,
  

•                                                       !!instances, "instance");
  

•                   if (!elements)
  

•                           exit(EXIT_FAILURE);
  

•                   instances = init_uls(elements, listcount, "instance");
  

•                   if (!instances)
  

•                           exit(EXIT_FAILURE);
  

•                   free(elements);
  

•                   elements = NULL;
                    break;
            case 'v':
  

•                   fmp_ph_params.fw_version = strtoul(optarg, NULL, 0);
  

•                   fmp_ph_params.have_header = true;
  

•                   elements = (const char **)init_list(&listcount, optarg, separator,
  

•                                                       !!fw_versions, "firmware version");
  

•                   if (!elements)
  

•                           exit(EXIT_FAILURE);
  

•                   fw_versions = init_uls(elements, listcount, "firmware version");
  

•                   if (!fw_versions)
  

•                           exit(EXIT_FAILURE);
  

•                   free(elements);
  

•                   elements = NULL;
                    break;
            case 'p':
                    if (privkey_file) {
  

@@ -730,7 +1014,17 @@ int main(int argc, char **argv) cert_file = optarg; break; case 'm':

•                   mcount = strtoul(optarg, NULL, 0);
  

•                   elements = (const char **)init_list(&listcount, optarg, separator,
  

•                                                       !!mcounts, "monotonic count");
  

•                   if (!elements)
  

•                           exit(EXIT_FAILURE);
  

•                   mcounts = init_uls(elements, listcount, "monotonic count");
  

•                   if (!mcounts)
  

•                           exit(EXIT_FAILURE);
  

•                   free(elements);
  

•                   elements = NULL;
                    break;
            case 'd':
                    dump_sig = 1;
  

@@ -767,26 +1061,46 @@ int main(int argc, char **argv)

    /* check necessary parameters */
    if ((capsule_type == CAPSULE_NORMAL_BLOB &&

•       ((argc != optind + 2) || !guid ||
  

•        ((privkey_file && !cert_file) ||
  

•         (!privkey_file && cert_file)))) ||
  

•        (!((argc != optind + 2) ^ !(blob_paths && argc == optind + 1)) || !guids ||
  

•         ((privkey_file && !cert_file) ||
  

•          (!privkey_file && cert_file)))) ||
        (capsule_type != CAPSULE_NORMAL_BLOB &&
  

•       ((argc != optind + 1) ||
  

•        ((capsule_type == CAPSULE_ACCEPT) && !guid) ||
  

•        ((capsule_type == CAPSULE_REVERT) && guid)))) {
  

•        ((argc != optind + 1) ||
  

•         ((capsule_type == CAPSULE_ACCEPT) && !guids) ||
  

•         ((capsule_type == CAPSULE_ACCEPT) && listcount != 1) ||
  

•         ((capsule_type == CAPSULE_REVERT) && guids)))) {
            print_usage();
            exit(EXIT_FAILURE);
    }
  
  

•   /* populate blob_paths if image blob was provided as positional argument */
  

•   if (capsule_type == CAPSULE_NORMAL_BLOB && !blob_paths) {
  

•           blob_paths = malloc(sizeof(char *));
  

•           if (!blob_paths) {
  

•                   fprintf(stderr, "Could not allocate memory for blob paths\n");
  

•                   exit(EXIT_FAILURE);
  

•           }
  

•           *blob_paths = argv[argc - 2];
  

•   }
  

•   /* populate arrays with zeros if they are not provided */
  

•   if (!indices)
  

•           indices = calloc(listcount, sizeof(unsigned long));
  

•   if (!instances)
  

•           instances = calloc(listcount, sizeof(unsigned long));
  

•   if (!mcounts)
  

•           mcounts = calloc(listcount, sizeof(uint64_t));
  

•   if (capsule_type != CAPSULE_NORMAL_BLOB) {
  

•           if (create_empty_capsule(argv[argc - 1], guid,
  

•           if (create_empty_capsule(argv[argc - 1], guids,
                                     capsule_type == CAPSULE_ACCEPT) < 0) {
                    fprintf(stderr, "Creating empty capsule failed\n");
                    exit(EXIT_FAILURE);
            }
  

•   } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
  

•                            index, instance, &fmp_ph_params, mcount, privkey_file,
  

•                            cert_file, (uint16_t)oemflags) < 0) {
  

•   } else if (create_fwbin(argv[argc - 1], blob_paths, guids,
  

•                           indices, instances, fw_versions,
  

•                           mcounts, listcount, privkey_file,
  

•                           cert_file, (uint16_t)oemflags) < 0) {
            fprintf(stderr, "Creating firmware capsule failed\n");
            exit(EXIT_FAILURE);
    }
  

-- 2.30.2

Hi Sughos,

one other question. Do you know what the advantage of mkeficapsule tool over the EDK2 GenerateCapsule tool is? It seems like reinventing the wheel for me.

Best Regards Malte

Am 16.06.2023 um 14:59 schrieb Schmidt, Malte:

Hi Sughos,

Am 16.06.2023 um 14:32 schrieb Sughosh Ganu:

...

On Fri, 16 Jun 2023 at 17:56, Sughosh Ganu sughosh.ganu@linaro.org wrote:

...

hi Stefan,

On Fri, 16 Jun 2023 at 17:04, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:

...

From: Malte Schmidt malte.schmidt@weidmueller.com

The UEFI [1] specification allows multiple payloads inside the capsule body. Add support for this. The command line arguments are kept backwards-compatible.

[1] https://uefi.org/specs/UEFI/2.10/index.html

I am trying to upstream support for specifying the capsule parameters for multiple payloads through a config file [1]. This is on similar lines to the support in the Edk2 GenerateCapule tool where multiple payloads can be specified through a json file. I think you can base your changes on my series.

Btw, with the support being added for getting the capsule parameters through a config file, I believe your changes would be pretty much simplified. Instead of passing all those parameters through the command line, they can instead be read from the config file and used to generate a single capsule file consisting of multiple payloads. That would be a much simpler implementation.

-sughosh

thanks for the heads up. So your opinion is that we only support multiple payloads via config files and not the command line? I think it does not hurt to have both options available.

I plan to rebase my code on yours once it nears the finish line. I still have a suggsetion for it which I will post in a sec.

Best Regards Malte

...

...

-sughosh

[1] - https://lore.kernel.org/u-boot/20230613103806.812065-1-sughosh.ganu@linaro.o...

...

Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com Signed-off-by: Stefan Herbrechtsmeier

stefan.herbrechtsmeier@weidmueller.com

tools/eficapsule.h   |   5 -   tools/mkeficapsule.c | 636 ++++++++++++++++++++++++++++++++-----------   2 files changed, 475 insertions(+), 166 deletions(-)

diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..001af3217c 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -138,9 +138,4 @@ struct fmp_payload_header {          uint32_t lowest_supported_version;   };

-struct fmp_payload_header_params { -       bool have_header; -       uint32_t fw_version; -};

#endif /* _EFI_CAPSULE_H */ diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index b8db00b16b..1a4de0f092 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule";   efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;

-static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR"; +static const char *opts_short = "g:i:b:I:v:p:c:m:o:dhAR";

enum {          CAPSULE_NORMAL_BLOB = 0, @@ -40,6 +40,7 @@ enum {   static struct option options[] = {          {"guid", required_argument, NULL, 'g'},          {"index", required_argument, NULL, 'i'}, +       {"image_blob", required_argument, NULL, 'b'},          {"instance", required_argument, NULL, 'I'},          {"fw-version", required_argument, NULL, 'v'},          {"private-key", required_argument, NULL, 'p'}, @@ -55,21 +56,22 @@ static struct option options[] = {

static void print_usage(void)   { -       fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n" +       fprintf(stderr, "Usage: %s [options] [<image blob>] <output file>\n"                  "Options:\n"

-               "\t-g, --guid <guid string>    guid for image blob type\n" -               "\t-i, --index <index>         update image index\n" -               "\t-I, --instance <instance>   update hardware instance\n" -               "\t-v, --fw-version <version> firmware version\n" -               "\t-p, --private-key <privkey file> private key file\n" -               "\t-c, --certificate <cert file> signer's certificate file\n" -               "\t-m, --monotonic-count <count> monotonic count\n" -               "\t-d, --dump_sig              dump signature (*.p7)\n" -               "\t-A, --fw-accept  firmware accept capsule, requires GUID, no image blob\n" -               "\t-R, --fw-revert  firmware revert capsule, takes no GUID, no image blob\n" -               "\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n" -               "\t-h, --help                  print a help message\n", +               "\t-g, --guid <guid list> comma-separated list of guids for image blob types\n" +               "\t-i, --index <index list> comma-separated list of update image indices\n" +               "\t-b, --image_blob <blob list> comma-separated list of image blobs\n" +               "\t-I, --instance <instance list> comma-separated list of update hardware instances\n" +               "\t-v, --fw-version <version list> comma-separated list of firmware versions\n" +               "\t-p, --private-key <privkey file>              private key file\n" +               "\t-c, --certificate <cert file>                 signer's certificate file\n" +               "\t-m, --monotonic-count <monotonic-count list>  comma-separated list of monotonic counts\n" +               "\t-d, --dump_sig                                dump signature (*.p7)\n" +               "\t-A, --fw-accept   firmware accept capsule, requires GUID, no image blob\n" +               "\t-R, --fw-revert   firmware revert capsule, takes no GUID, no image blob\n" +               "\t-o, --capoemflag  capsule OEM Flag, an integer between 0x0000 and 0xffff\n" +               "\t-h, --help        print a help message\n",                  tool_name);   }

@@ -336,16 +338,18 @@ static int create_auth_data(struct auth_context *ctx)    * @path:      Path to a capsule file    * @signature: Signature data    * @sig_size:  Size of signature data

• • @index:     The payload index the signature belongs to

*    * Signature data pointed to by @signature will be saved into

• • a file whose file name is @path with ".p7" suffix.

• • a file whose file name is @path with "_<index>.p7" suffix.
• • If index is negative the suffix is ".p7" (for backwards

compatibility).    *    * Return:    * * 0  - on success    * * -1 - on failure    */   static int dump_signature(const char *path, const uint8_t *signature, -                         size_t sig_size) +                         size_t sig_size, int index)   {          char *sig_path;          FILE *f; @@ -356,7 +360,11 @@ static int dump_signature(const char *path, const uint8_t *signature,          if (!sig_path)                  return ret;

-       sprintf(sig_path, "%s.p7", path); +       if (index < 0) +               sprintf(sig_path, "%s.p7", path); +       else +               sprintf(sig_path, "%s_%d.p7", path, index);

f = fopen(sig_path, "w");          if (!f)                  goto err; @@ -386,14 +394,15 @@ static void free_sig_data(struct auth_context *ctx)   /**    * create_fwbin - create an uefi capsule file    * @path:      Path to a created capsule file

• • @bin:       Path to a firmware binary to encapsulate
• • @guid:      GUID of related FMP driver
• • @index:     Index number in capsule

• • @bins:      Paths to firmware binaries to encapsulate, an array
• • @guids:     GUIDs of related FMP drivers, an array
• • @indices:   Index numbers in capsule, an array

* @instance:  Instance number in capsule    * @mcount:    Monotonic count in authentication information

• • @size:      Size of the arrays

* @private_file:      Path to a private key file    * @cert_file: Path to a certificate file

• • @oemflags:  Capsule OEM Flags, bits 0-15

• • @oemflags:  Capsule OEM Flags, bits 0-15

*    * This function actually does the job of creating an uefi capsule file.    * All the arguments must be supplied. @@ -404,78 +413,87 @@ static void free_sig_data(struct auth_context *ctx)    * * 0  - on success    * * -1 - on failure    */ -static int create_fwbin(const char *path, const char *bin, -                       const efi_guid_t *guid, unsigned long index, -                       unsigned long instance, -                       const struct fmp_payload_header_params *fmp_ph_params, -                       uint64_t mcount, -                       const char *privkey_file, const char *cert_file, -                       uint16_t oemflags) +static int create_fwbin(const char *path, const char **bins, +                       const efi_guid_t *guids, const unsigned long *indices, +                       const unsigned long *instances, +                       const unsigned long *fw_versions, const unsigned long *mcounts, +                       int size, const char *privkey_file, +                       const char *cert_file, uint16_t oemflags)   {          struct efi_capsule_header header;          struct efi_firmware_management_capsule_header capsule; -       struct efi_firmware_management_capsule_image_header image; -       struct auth_context auth_context; +       struct efi_firmware_management_capsule_image_header images[size]; +       struct auth_context auth_contexts[size];          FILE *f; -       uint8_t *data, *new_data, *buf; -       off_t bin_size; -       uint64_t offset; +       uint8_t *data_list[size], *new_data_list[size], *buf_list[size]; +       off_t bin_sizes[size]; +       uint64_t offsets[size];          int ret; -       struct fmp_payload_header payload_header; +       struct fmp_payload_header payload_headers[size];

#ifdef DEBUG          fprintf(stderr, "For output: %s\n", path); -       fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid); -       fprintf(stderr, "\tindex: %lu\n\tinstance: %lu\n", index, instance); +       for (int i = 0; i < size; i++) { +               fprintf(stderr, "\tpayload no: %d\n", i); +               fprintf(stderr, "\t\tbin: %s\n\t\ttype: %pUl\n", bins[i], guids[i]); +               fprintf(stderr, "\t\tindex: %lu\n\t\tinstance: %lu\n", indices[i], instances[i]); +       }   #endif -       auth_context.sig_size = 0;          f = NULL; -       data = NULL; -       new_data = NULL;          ret = -1;

-       /* -        * read a firmware binary -        */ -       if (read_bin_file(bin, &data, &bin_size)) -               goto err; +       for (int i = 0; i < size; i++) { +               auth_contexts[i].sig_size = 0; +               data_list[i] = NULL; +               new_data_list[i] = NULL; +       }

-       buf = data; +       for (int i = 0; i < size; i++) { +               int dump_index = (size == 1) ? -1 : i;

-       /* insert fmp payload header right before the payload */ -       if (fmp_ph_params->have_header) { -               new_data = malloc(bin_size + sizeof(payload_header)); -               if (!new_data) +               /* +                * read a firmware binary +                */ +               if (read_bin_file(bins[i], &data_list[i], &bin_sizes[i]))                          goto err;

-               payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE; -               payload_header.header_size = sizeof(payload_header); -               payload_header.fw_version = fmp_ph_params->fw_version; -               payload_header.lowest_supported_version = 0; /* not used */ -               memcpy(new_data, &payload_header, sizeof(payload_header)); -               memcpy(new_data + sizeof(payload_header), data, bin_size); -               buf = new_data; -               bin_size += sizeof(payload_header); -       }

-       /* first, calculate signature to determine its size */ -       if (privkey_file && cert_file) { -               auth_context.key_file = privkey_file; -               auth_context.cert_file = cert_file; -               auth_context.auth.monotonic_count = mcount; -               auth_context.image_data = buf; -               auth_context.image_size = bin_size;

-               if (create_auth_data(&auth_context)) { -                       fprintf(stderr, "Signing firmware image failed\n"); -                       goto err; +               buf_list[i] = data_list[i]; +               /* insert fmp payload header right before the payload */ +               if (fw_versions) { +                       new_data_list[i] = malloc(bin_sizes[i] + sizeof(payload_headers[i])); +                       if (!new_data_list[i]) +                               goto err;

+                       payload_headers[i].signature = FMP_PAYLOAD_HDR_SIGNATURE; +                       payload_headers[i].header_size = sizeof(payload_headers[i]); +                       payload_headers[i].fw_version = fw_versions[i];

• payload_headers[i].lowest_supported_version = 0; /* not used */

+                       memcpy(new_data_list[i], (payload_headers + i), sizeof(payload_headers[i])); +                       memcpy(new_data_list[i] + sizeof(payload_headers[i]), data_list[i], +                              bin_sizes[i]); +                       buf_list[i] = new_data_list[i]; +                       bin_sizes[i] += sizeof(payload_headers[i]);                  }

-               if (dump_sig && -                   dump_signature(path, auth_context.sig_data, -                                  auth_context.sig_size)) { -                       fprintf(stderr, "Creating signature file failed\n"); -                       goto err; +               /* calculate signature to determine its size */ +               if (privkey_file && cert_file) { +                       auth_contexts[i].key_file = privkey_file; +                       auth_contexts[i].cert_file = cert_file;

• auth_contexts[i].auth.monotonic_count = mcounts[i];

+                       auth_contexts[i].image_data = buf_list[i]; +                       auth_contexts[i].image_size = bin_sizes[i];

+                       if (create_auth_data(&auth_contexts[i])) { +                               fprintf(stderr, "Signing firmware image failed\n"); +                               goto err; +                       }

+                       if (dump_sig && +                           dump_signature(path, auth_contexts[i].sig_data,

• auth_contexts[i].sig_size, dump_index)) {

+                               fprintf(stderr, "Creating signature file failed\n"); +                               goto err; +                       }                  }          }

@@ -498,81 +516,87 @@ static int create_fwbin(const char *path, const char *bin,          if (oemflags)                  header.flags |= oemflags;          header.capsule_image_size = sizeof(header) -                                       + sizeof(capsule) + sizeof(uint64_t) -                                       + sizeof(image) -                                       + bin_size; -       if (auth_context.sig_size) -               header.capsule_image_size += sizeof(auth_context.auth) -                               + auth_context.sig_size; +                                       + sizeof(capsule) +                                       + size * sizeof(uint64_t); /* size of item_offset_list */ +       for (int i = 0; i < size; i++) { +               offsets[i] = header.capsule_image_size - sizeof(header); +               header.capsule_image_size += sizeof(images[i]) +                                       + bin_sizes[i]; +               if (auth_contexts[i].sig_size) +                       header.capsule_image_size += sizeof(auth_contexts[i].auth) +                                       + auth_contexts[i].sig_size; +       }          if (write_capsule_file(f, &header, sizeof(header),                                 "Capsule header"))                  goto err;

/*           * firmware capsule header -        * This capsule has only one firmware capsule image.           */          capsule.version = 0x00000001;          capsule.embedded_driver_count = 0; -       capsule.payload_item_count = 1; +       capsule.payload_item_count = size;          if (write_capsule_file(f, &capsule, sizeof(capsule),                                 "Firmware capsule header"))                  goto err;

-       offset = sizeof(capsule) + sizeof(uint64_t); -       if (write_capsule_file(f, &offset, sizeof(offset), -                              "Offset to capsule image")) +       if (write_capsule_file(f, &offsets, size * sizeof(uint64_t), +                              "Offsets to capsule images"))                  goto err;

-       /* -        * firmware capsule image header -        */ -       image.version = 0x00000003; -       memcpy(&image.update_image_type_id, guid, sizeof(*guid)); -       image.update_image_index = index; -       image.reserved[0] = 0; -       image.reserved[1] = 0; -       image.reserved[2] = 0; -       image.update_image_size = bin_size; -       if (auth_context.sig_size) -               image.update_image_size += sizeof(auth_context.auth) -                               + auth_context.sig_size; -       image.update_vendor_code_size = 0; /* none */ -       image.update_hardware_instance = instance; -       image.image_capsule_support = 0; -       if (auth_context.sig_size) -               image.image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION; -       if (write_capsule_file(f, &image, sizeof(image), -                              "Firmware capsule image header")) -               goto err;

-       /* -        * signature -        */ -       if (auth_context.sig_size) { -               if (write_capsule_file(f, &auth_context.auth,

• sizeof(auth_context.auth),

-                                      "Authentication header")) +       for (int i = 0; i < size; i++) { +               /* +                * firmware capsule image header +                */ +               images[i].version = 0x00000003; +               memcpy(&images[i].update_image_type_id, &guids[i], sizeof(guids[i])); +               images[i].update_image_index = indices[i]; +               images[i].reserved[0] = 0; +               images[i].reserved[1] = 0; +               images[i].reserved[2] = 0; +               images[i].update_image_size = bin_sizes[i]; +               if (auth_contexts[i].sig_size) +                       images[i].update_image_size += sizeof(auth_contexts[i].auth) +                                       + auth_contexts[i].sig_size; +               images[i].update_vendor_code_size = 0; /* none */ +               images[i].update_hardware_instance = instances[i]; +               images[i].image_capsule_support = 0; +               if (auth_contexts[i].sig_size) +                       images[i].image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION; +               if (write_capsule_file(f, &images[i], sizeof(images[i]), +                                      "Firmware capsule image header"))                          goto err;

-               if (write_capsule_file(f, auth_context.sig_data,

• auth_context.sig_size, "Signature"))

+               /* +                * signature +                */ +               if (auth_contexts[i].sig_size) { +                       if (write_capsule_file(f, &auth_contexts[i].auth,

• sizeof(auth_contexts[i].auth),
• "Authentication header"))

+                               goto err;

+                       if (write_capsule_file(f, auth_contexts[i].sig_data,

• auth_contexts[i].sig_size, "Signature"))

+                               goto err; +               }

+               /* +                * firmware binary +                */ +               if (write_capsule_file(f, buf_list[i], bin_sizes[i], "Firmware binary"))                          goto err;          }

-       /* -        * firmware binary -        */ -       if (write_capsule_file(f, buf, bin_size, "Firmware binary")) -               goto err;

ret = 0;   err:          if (f)                  fclose(f); -       free_sig_data(&auth_context); -       free(data); -       free(new_data); +       for (int i = 0; i < size; i++) { +               free_sig_data(&auth_contexts[i]); +               free(data_list[i]); +               free(new_data_list[i]); +       }

return ret;   } @@ -652,6 +676,228 @@ err:          return ret;   }

+/**

• • count_items - count number of items in list
• • @list:      Pointer to a string
• • @separator: Separator used to separate list items
• • Count the number of items in a list. The list items
• • are separated by a separator character inside the string.
• • Trailing white spaces are not allowed except if it is the

separator.

• • Return:
• • The item count.
• */

+int count_items(const char *list, char separator) +{ +       const char *c; +       int count = 0;

+       if (!*list) +               return 0;

+       for (c = list; *c; c++) { +               if (*c == separator) +                       count++; +       } +       /* correct count if no trailing separator present */ +       if (*(c - 1) != separator) +               count++;

+       return count; +}

+/**

• • update_itemcount - update item count
• • @count:     The count to be updated
• • @list:      The item list
• • @separator: List separator
• • Initialize the count if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with the

list count.

• • Return:
• • • 0 - on success
• • • -1 - if a check fails
• */

+int update_itemcount(int *count, const char *list, char separator) +{ +       int current_count = count_items(list, separator);

+       if (*count < 0) +               *count = current_count;

+       if (*count == 0 || +           *count != current_count) +               return -1;

+       return 0; +}

+/**

• • split_list - split list into elements
• • @elements:  Pointer to string array
• • @size:      The array size
• • @list:      The item list
• • @separator: List separator
• • Split a comma-separated list into its elements.
• • Return:
• • • 0 - on success
• • • -1 - on failure
• */

+int split_list(char **elements, int size, char *list, char separator) +{ +       const char separator_str[] = {separator, '\0'}; +       char *end;

+       for (int i = 0; i < size; i++) { +               elements[i] = strsep(&list, separator_str); +               if (!elements[i]) +                       return -1; +       }

+       end = strsep(&list, separator_str);  /* NULL or empty string expected */ +       if (end && *end) +               return -1;

+       return 0; +}

+/**

• • alloc_array - allocate memory for array
• • @count:     The number of elements
• • @obj_size:  The size of a single element
• • @name:      The name of the array
• • This is a wrapper for malloc which prints an error
• • message on failure.
• *  Return:
• • • Pointer to the allocated memory on success
• • • NULL on failure
• */

+void *alloc_array(unsigned int count, size_t obj_size, const char *name) +{ +       void *array;

+       array = malloc(count * obj_size); +       if (!array) +               fprintf(stderr, "Could not allocate memory for %s\n", name);

+       return array; +}

+/**

• • init_guids - populate guid array
• • @elements:  String array of elements to be converted
• • @size:      The array size
• • @name:      The name of the array
• • Allocate and populate an array of guid structs. The list

contains the UUIDs

• • to convert and store in the array. Upon failure an error

message is

• • printed.
• • Return:
• • • The initialized GUID array on success
• • • NULL on failure
• */

+efi_guid_t *init_guids(const char **elements, unsigned int size, +                      const char *name) +{ +       efi_guid_t *guids;

+       guids = alloc_array(size, sizeof(efi_guid_t), name); +       if (!guids) +               return NULL;

+       for (int i = 0; i < size; i++) { +               if (uuid_parse(elements[i], (unsigned char *)(guids

• i))) {

+                       fprintf(stderr, "Wrong %s format\n", name); +                       free(guids); +                       return NULL; +               } +               convert_uuid_to_guid((unsigned char *)(guids + i)); +       }

+       return guids; +}

+/**

• • init_uls - populate unsigned long array
• • @elements:  String array of elements to be converted
• • @size:      The array size
• • @name:      The name of the array
• • Allocate and populate an array of unsgined longs. Upon failure an
• • error message is printed.
• • Return:
• • • The initialized array on success
• • • NULL on failure
• */

+unsigned long *init_uls(const char **elements, unsigned int size, +                       const char *name) +{ +       unsigned long *array;

+       array = alloc_array(size, sizeof(unsigned long), name); +       if (!array) +               return NULL; +       for (int i = 0; i < size; i++) +               array[i] = strtoul(elements[i], NULL, 0);

+       return array; +}

+/**

• • init_list - parse list and allocate elements
• • @listcount: The list count to be checked and updated
• • @list:      The list to be parsed
• • @separator: The list separator
• • @name:      The name of the list
• • @multiple_times:    List encountered multiple times
• • Routine for command line argument lists.
• • Parse the string list and count the list elements.
• • Initialize the listcount if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with the

list count.

• • Allocate the string array and populate it with the list elements.
• • The array should be freed in the calling function.
• • Upon failure an error message is printed and the program exits.
• *  Return:
• • • The initialized array on success
• • • NULL on failure
• */

+char **init_list(int *listcount, char *list, char separator, +                bool multiple_times, char *name) +{ +       char **elements;

+       if (multiple_times) { +               fprintf(stderr, "%s specified multiple times\n", name); +               return NULL; +       } +       if (update_itemcount(listcount, list, separator)) { +               fprintf(stderr, "List count not consistent with previous or list not provided\n"); +               return NULL; +       } +       elements = alloc_array(*listcount, sizeof(char *), name); +       if (!elements) +               return NULL; +       if (split_list(elements, *listcount, list, separator)) { +               fprintf(stderr, "Could not parse %s list\n", name); +               free(elements); +               return NULL; +       }

+       return elements; +}

/**    * main - main entry function of mkeficapsule    * @argc:      Number of arguments @@ -666,24 +912,27 @@ err:    */   int main(int argc, char **argv)   { -       efi_guid_t *guid; -       unsigned char uuid_buf[16]; -       unsigned long index, instance; -       uint64_t mcount; +       const char separator = ','; +       const efi_guid_t *guids; /* an array */ +       const unsigned long *indices, *instances, *mcounts, *fw_versions; /* arrays */          unsigned long oemflags; +       const char **blob_paths, **elements;  /* string arrays */          const char *privkey_file, *cert_file; -       int c, idx; -       struct fmp_payload_header_params fmp_ph_params = { 0 }; +       int listcount, c, idx;

-       guid = NULL; -       index = 0; -       instance = 0; -       mcount = 0; +       guids = NULL; +       indices = NULL; +       instances = NULL; +       mcounts = NULL; +       oemflags = 0; +       blob_paths = NULL;          privkey_file = NULL;          cert_file = NULL; +       elements = NULL; +       listcount = -1; +       fw_versions = NULL;          dump_sig = 0;          capsule_type = CAPSULE_NORMAL_BLOB; -       oemflags = 0;          for (;;) {                  c = getopt_long(argc, argv, opts_short, options, &idx);                  if (c == -1) @@ -691,27 +940,62 @@ int main(int argc, char **argv)

switch (c) {                  case 'g': -                       if (guid) { -                               fprintf(stderr, -                                       "Image type already specified\n"); +                       elements = (const char **)init_list(&listcount, optarg, separator, !!guids,

• "GUID");

+                       if (!elements)                                  exit(EXIT_FAILURE); -                       } -                       if (uuid_parse(optarg, uuid_buf)) { -                               fprintf(stderr, "Wrong guid format\n");

+                       guids = init_guids(elements, listcount, "GUID"); +                       if (!guids)                                  exit(EXIT_FAILURE); -                       } -                       convert_uuid_to_guid(uuid_buf); -                       guid = (efi_guid_t *)uuid_buf;

+                       free(elements); +                       elements = NULL;                          break;                  case 'i': -                       index = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!indices, "index");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       indices = init_uls(elements, listcount, "index"); +                       if (!indices) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL; +                       break; +               case 'b': +                       blob_paths = (const char **)init_list(&listcount, optarg, separator,

• !!blob_paths, "blob path");

+                       if (!blob_paths) +                               exit(EXIT_FAILURE);                          break;                  case 'I': -                       instance = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!instances, "instance");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       instances = init_uls(elements, listcount, "instance"); +                       if (!instances) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'v': -                       fmp_ph_params.fw_version = strtoul(optarg, NULL, 0); -                       fmp_ph_params.have_header = true; +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!fw_versions, "firmware version");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       fw_versions = init_uls(elements, listcount, "firmware version"); +                       if (!fw_versions) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'p':                          if (privkey_file) { @@ -730,7 +1014,17 @@ int main(int argc, char **argv)                          cert_file = optarg;                          break;                  case 'm': -                       mcount = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!mcounts, "monotonic count");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       mcounts = init_uls(elements, listcount, "monotonic count"); +                       if (!mcounts) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'd':                          dump_sig = 1; @@ -767,26 +1061,46 @@ int main(int argc, char **argv)

/* check necessary parameters */          if ((capsule_type == CAPSULE_NORMAL_BLOB && -           ((argc != optind + 2) || !guid || -            ((privkey_file && !cert_file) || -             (!privkey_file && cert_file)))) || +            (!((argc != optind + 2) ^ !(blob_paths && argc == optind + 1)) || !guids || +             ((privkey_file && !cert_file) || +              (!privkey_file && cert_file)))) ||              (capsule_type != CAPSULE_NORMAL_BLOB && -           ((argc != optind + 1) || -            ((capsule_type == CAPSULE_ACCEPT) && !guid) || -            ((capsule_type == CAPSULE_REVERT) && guid)))) { +            ((argc != optind + 1) || +             ((capsule_type == CAPSULE_ACCEPT) && !guids) || +             ((capsule_type == CAPSULE_ACCEPT) && listcount != 1) || +             ((capsule_type == CAPSULE_REVERT) && guids)))) {                  print_usage();                  exit(EXIT_FAILURE);          }

+       /* populate blob_paths if image blob was provided as positional argument */ +       if (capsule_type == CAPSULE_NORMAL_BLOB && !blob_paths) { +               blob_paths = malloc(sizeof(char *)); +               if (!blob_paths) { +                       fprintf(stderr, "Could not allocate memory for blob paths\n"); +                       exit(EXIT_FAILURE); +               } +               *blob_paths = argv[argc - 2]; +       }

+       /* populate arrays with zeros if they are not provided */ +       if (!indices) +               indices = calloc(listcount, sizeof(unsigned long)); +       if (!instances) +               instances = calloc(listcount, sizeof(unsigned long)); +       if (!mcounts) +               mcounts = calloc(listcount, sizeof(uint64_t));

if (capsule_type != CAPSULE_NORMAL_BLOB) { -               if (create_empty_capsule(argv[argc - 1], guid, +               if (create_empty_capsule(argv[argc - 1], guids,                                           capsule_type == CAPSULE_ACCEPT) < 0) {                          fprintf(stderr, "Creating empty capsule failed\n");                          exit(EXIT_FAILURE);                  } -       } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid, -                                index, instance, &fmp_ph_params, mcount, privkey_file, -                                cert_file, (uint16_t)oemflags) < 0) { +       } else if (create_fwbin(argv[argc - 1], blob_paths, guids, +                               indices, instances, fw_versions, +                               mcounts, listcount, privkey_file, +                               cert_file, (uint16_t)oemflags) < 0) {                  fprintf(stderr, "Creating firmware capsule failed\n");                  exit(EXIT_FAILURE);          } -- 2.30.2

Hello Takahiro,

Am 17.06.2023 um 02:37 schrieb AKASHI Takahiro:

On Fri, Jun 16, 2023 at 03:32:59PM +0200, Schmidt, Malte wrote:

...

Hi Sughos,

one other question. Do you know what the advantage of mkeficapsule tool over the EDK2 GenerateCapsule tool is? It seems like reinventing the wheel for me.

Simply because I didn't want to have a dependency on EDK2 package when users use U-Boot as EFI framework. (In other words, make it self-contained?)

-Takahiro Akashi

thanks for the clarification.

Best Regards Malte

...

Best Regards Malte

Am 16.06.2023 um 14:59 schrieb Schmidt, Malte:

...

Hi Sughos,

Am 16.06.2023 um 14:32 schrieb Sughosh Ganu:

...

On Fri, 16 Jun 2023 at 17:56, Sughosh Ganu sughosh.ganu@linaro.org wrote:

...

hi Stefan,

On Fri, 16 Jun 2023 at 17:04, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:

...

From: Malte Schmidt malte.schmidt@weidmueller.com

The UEFI [1] specification allows multiple payloads inside the capsule body. Add support for this. The command line arguments are kept backwards-compatible.

[1] https://uefi.org/specs/UEFI/2.10/index.html

I am trying to upstream support for specifying the capsule parameters for multiple payloads through a config file [1]. This is on similar lines to the support in the Edk2 GenerateCapule tool where multiple payloads can be specified through a json file. I think you can base your changes on my series.

Btw, with the support being added for getting the capsule parameters through a config file, I believe your changes would be pretty much simplified. Instead of passing all those parameters through the command line, they can instead be read from the config file and used to generate a single capsule file consisting of multiple payloads. That would be a much simpler implementation.

-sughosh

thanks for the heads up. So your opinion is that we only support multiple payloads via config files and not the command line? I think it does not hurt to have both options available.

I plan to rebase my code on yours once it nears the finish line. I still have a suggsetion for it which I will post in a sec.

Best Regards Malte

...

...

-sughosh

[1] - https://lore.kernel.org/u-boot/20230613103806.812065-1-sughosh.ganu@linaro.o...

...

Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com Signed-off-by: Stefan Herbrechtsmeier

stefan.herbrechtsmeier@weidmueller.com

tools/eficapsule.h   |   5 -   tools/mkeficapsule.c | 636 ++++++++++++++++++++++++++++++++-----------   2 files changed, 475 insertions(+), 166 deletions(-)

diff --git a/tools/eficapsule.h b/tools/eficapsule.h index 753fb73313..001af3217c 100644 --- a/tools/eficapsule.h +++ b/tools/eficapsule.h @@ -138,9 +138,4 @@ struct fmp_payload_header {          uint32_t lowest_supported_version;   };

-struct fmp_payload_header_params { -       bool have_header; -       uint32_t fw_version; -};

#endif /* _EFI_CAPSULE_H */ diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index b8db00b16b..1a4de0f092 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -29,7 +29,7 @@ static const char *tool_name = "mkeficapsule";   efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;

-static const char *opts_short = "g:i:I:v:p:c:m:o:dhAR"; +static const char *opts_short = "g:i:b:I:v:p:c:m:o:dhAR";

enum {          CAPSULE_NORMAL_BLOB = 0, @@ -40,6 +40,7 @@ enum {   static struct option options[] = {          {"guid", required_argument, NULL, 'g'},          {"index", required_argument, NULL, 'i'}, +       {"image_blob", required_argument, NULL, 'b'},          {"instance", required_argument, NULL, 'I'},          {"fw-version", required_argument, NULL, 'v'},          {"private-key", required_argument, NULL, 'p'}, @@ -55,21 +56,22 @@ static struct option options[] = {

static void print_usage(void)   { -       fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n" +       fprintf(stderr, "Usage: %s [options] [<image blob>] <output file>\n"                  "Options:\n"

-               "\t-g, --guid <guid string>    guid for image blob type\n" -               "\t-i, --index <index>         update image index\n" -               "\t-I, --instance <instance>   update hardware instance\n" -               "\t-v, --fw-version <version> firmware version\n" -               "\t-p, --private-key <privkey file> private key file\n" -               "\t-c, --certificate <cert file> signer's certificate file\n" -               "\t-m, --monotonic-count <count> monotonic count\n" -               "\t-d, --dump_sig              dump signature (*.p7)\n" -               "\t-A, --fw-accept  firmware accept capsule, requires GUID, no image blob\n" -               "\t-R, --fw-revert  firmware revert capsule, takes no GUID, no image blob\n" -               "\t-o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff\n" -               "\t-h, --help                  print a help message\n", +               "\t-g, --guid <guid list> comma-separated list of guids for image blob types\n" +               "\t-i, --index <index list> comma-separated list of update image indices\n" +               "\t-b, --image_blob <blob list> comma-separated list of image blobs\n" +               "\t-I, --instance <instance list> comma-separated list of update hardware instances\n" +               "\t-v, --fw-version <version list> comma-separated list of firmware versions\n" +               "\t-p, --private-key <privkey file>              private key file\n" +               "\t-c, --certificate <cert file>                 signer's certificate file\n" +               "\t-m, --monotonic-count <monotonic-count list>  comma-separated list of monotonic counts\n" +               "\t-d, --dump_sig dump signature (*.p7)\n" +               "\t-A, --fw-accept firmware accept capsule, requires GUID, no image blob\n" +               "\t-R, --fw-revert firmware revert capsule, takes no GUID, no image blob\n" +               "\t-o, --capoemflag  capsule OEM Flag, an integer between 0x0000 and 0xffff\n" +               "\t-h, --help        print a help message\n",                  tool_name);   }

@@ -336,16 +338,18 @@ static int create_auth_data(struct auth_context *ctx)    * @path:      Path to a capsule file    * @signature: Signature data    * @sig_size:  Size of signature data

• • @index:     The payload index the signature belongs to

*    * Signature data pointed to by @signature will be saved into

• • a file whose file name is @path with ".p7" suffix.

• • a file whose file name is @path with "_<index>.p7" suffix.
• • If index is negative the suffix is ".p7" (for backwards

compatibility).    *    * Return:    * * 0  - on success    * * -1 - on failure    */   static int dump_signature(const char *path, const uint8_t *signature, -                         size_t sig_size) +                         size_t sig_size, int index)   {          char *sig_path;          FILE *f; @@ -356,7 +360,11 @@ static int dump_signature(const char *path, const uint8_t *signature,          if (!sig_path)                  return ret;

-       sprintf(sig_path, "%s.p7", path); +       if (index < 0) +               sprintf(sig_path, "%s.p7", path); +       else +               sprintf(sig_path, "%s_%d.p7", path, index);

f = fopen(sig_path, "w");          if (!f)                  goto err; @@ -386,14 +394,15 @@ static void free_sig_data(struct auth_context *ctx)   /**    * create_fwbin - create an uefi capsule file    * @path:      Path to a created capsule file

• • @bin:       Path to a firmware binary to encapsulate
• • @guid:      GUID of related FMP driver
• • @index:     Index number in capsule

• • @bins:      Paths to firmware binaries to encapsulate, an array
• • @guids:     GUIDs of related FMP drivers, an array
• • @indices:   Index numbers in capsule, an array

* @instance:  Instance number in capsule    * @mcount:    Monotonic count in authentication information

• • @size:      Size of the arrays

* @private_file:      Path to a private key file    * @cert_file: Path to a certificate file

• • @oemflags:  Capsule OEM Flags, bits 0-15

• • @oemflags:  Capsule OEM Flags, bits 0-15

*    * This function actually does the job of creating an uefi capsule file.    * All the arguments must be supplied. @@ -404,78 +413,87 @@ static void free_sig_data(struct auth_context *ctx)    * * 0  - on success    * * -1 - on failure    */ -static int create_fwbin(const char *path, const char *bin, -                       const efi_guid_t *guid, unsigned long index, -                       unsigned long instance, -                       const struct fmp_payload_header_params *fmp_ph_params, -                       uint64_t mcount, -                       const char *privkey_file, const char *cert_file, -                       uint16_t oemflags) +static int create_fwbin(const char *path, const char **bins, +                       const efi_guid_t *guids, const unsigned long *indices, +                       const unsigned long *instances, +                       const unsigned long *fw_versions, const unsigned long *mcounts, +                       int size, const char *privkey_file, +                       const char *cert_file, uint16_t oemflags)   {          struct efi_capsule_header header;          struct efi_firmware_management_capsule_header capsule; -       struct efi_firmware_management_capsule_image_header image; -       struct auth_context auth_context; +       struct efi_firmware_management_capsule_image_header images[size]; +       struct auth_context auth_contexts[size];          FILE *f; -       uint8_t *data, *new_data, *buf; -       off_t bin_size; -       uint64_t offset; +       uint8_t *data_list[size], *new_data_list[size], *buf_list[size]; +       off_t bin_sizes[size]; +       uint64_t offsets[size];          int ret; -       struct fmp_payload_header payload_header; +       struct fmp_payload_header payload_headers[size];

#ifdef DEBUG          fprintf(stderr, "For output: %s\n", path); -       fprintf(stderr, "\tbin: %s\n\ttype: %pUl\n", bin, guid); -       fprintf(stderr, "\tindex: %lu\n\tinstance: %lu\n", index, instance); +       for (int i = 0; i < size; i++) { +               fprintf(stderr, "\tpayload no: %d\n", i); +               fprintf(stderr, "\t\tbin: %s\n\t\ttype: %pUl\n", bins[i], guids[i]); +               fprintf(stderr, "\t\tindex: %lu\n\t\tinstance: %lu\n", indices[i], instances[i]); +       }   #endif -       auth_context.sig_size = 0;          f = NULL; -       data = NULL; -       new_data = NULL;          ret = -1;

-       /* -        * read a firmware binary -        */ -       if (read_bin_file(bin, &data, &bin_size)) -               goto err; +       for (int i = 0; i < size; i++) { +               auth_contexts[i].sig_size = 0; +               data_list[i] = NULL; +               new_data_list[i] = NULL; +       }

-       buf = data; +       for (int i = 0; i < size; i++) { +               int dump_index = (size == 1) ? -1 : i;

-       /* insert fmp payload header right before the payload */ -       if (fmp_ph_params->have_header) { -               new_data = malloc(bin_size + sizeof(payload_header)); -               if (!new_data) +               /* +                * read a firmware binary +                */ +               if (read_bin_file(bins[i], &data_list[i], &bin_sizes[i]))                          goto err;

-               payload_header.signature = FMP_PAYLOAD_HDR_SIGNATURE; -               payload_header.header_size = sizeof(payload_header); -               payload_header.fw_version = fmp_ph_params->fw_version;

payload_header.lowest_supported_version = 0; /* not used */ -               memcpy(new_data, &payload_header, sizeof(payload_header)); -               memcpy(new_data + sizeof(payload_header), data, bin_size); -               buf = new_data; -               bin_size += sizeof(payload_header); -       }

-       /* first, calculate signature to determine its size */ -       if (privkey_file && cert_file) { -               auth_context.key_file = privkey_file; -               auth_context.cert_file = cert_file; -               auth_context.auth.monotonic_count = mcount; -               auth_context.image_data = buf; -               auth_context.image_size = bin_size;

-               if (create_auth_data(&auth_context)) {

fprintf(stderr, "Signing firmware image failed\n"); -                       goto err; +               buf_list[i] = data_list[i]; +               /* insert fmp payload header right before the payload */ +               if (fw_versions) {

new_data_list[i] = malloc(bin_sizes[i] + sizeof(payload_headers[i])); +                       if (!new_data_list[i]) +                               goto err;

payload_headers[i].signature = FMP_PAYLOAD_HDR_SIGNATURE;

payload_headers[i].header_size = sizeof(payload_headers[i]);

payload_headers[i].fw_version = fw_versions[i];

• payload_headers[i].lowest_supported_version = 0; /* not used */

memcpy(new_data_list[i], (payload_headers + i), sizeof(payload_headers[i]));

memcpy(new_data_list[i] + sizeof(payload_headers[i]), data_list[i], +                              bin_sizes[i]); +                       buf_list[i] = new_data_list[i]; +                       bin_sizes[i] += sizeof(payload_headers[i]);                  }

-               if (dump_sig && -                   dump_signature(path, auth_context.sig_data, -                                  auth_context.sig_size)) {

fprintf(stderr, "Creating signature file failed\n"); -                       goto err; +               /* calculate signature to determine its size */ +               if (privkey_file && cert_file) { +                       auth_contexts[i].key_file = privkey_file; +                       auth_contexts[i].cert_file = cert_file;

• auth_contexts[i].auth.monotonic_count = mcounts[i];

+                       auth_contexts[i].image_data = buf_list[i]; +                       auth_contexts[i].image_size = bin_sizes[i];

+                       if (create_auth_data(&auth_contexts[i])) {

fprintf(stderr, "Signing firmware image failed\n"); +                               goto err; +                       }

+                       if (dump_sig &&

dump_signature(path, auth_contexts[i].sig_data,

• auth_contexts[i].sig_size, dump_index)) {

fprintf(stderr, "Creating signature file failed\n"); +                               goto err; +                       }                  }          }

@@ -498,81 +516,87 @@ static int create_fwbin(const char *path, const char *bin,          if (oemflags)                  header.flags |= oemflags;          header.capsule_image_size = sizeof(header)

• sizeof(capsule) + sizeof(uint64_t)

-                                       + sizeof(image) -                                       + bin_size; -       if (auth_context.sig_size) -               header.capsule_image_size += sizeof(auth_context.auth) -                               + auth_context.sig_size; +                                       + sizeof(capsule)

• size * sizeof(uint64_t); /* size of item_offset_list */

+       for (int i = 0; i < size; i++) { +               offsets[i] = header.capsule_image_size - sizeof(header); +               header.capsule_image_size += sizeof(images[i]) +                                       + bin_sizes[i]; +               if (auth_contexts[i].sig_size)

header.capsule_image_size += sizeof(auth_contexts[i].auth) +                                       + auth_contexts[i].sig_size; +       }          if (write_capsule_file(f, &header, sizeof(header),                                 "Capsule header"))                  goto err;

/*           * firmware capsule header -        * This capsule has only one firmware capsule image.           */          capsule.version = 0x00000001;          capsule.embedded_driver_count = 0; -       capsule.payload_item_count = 1; +       capsule.payload_item_count = size;          if (write_capsule_file(f, &capsule, sizeof(capsule),                                 "Firmware capsule header"))                  goto err;

-       offset = sizeof(capsule) + sizeof(uint64_t); -       if (write_capsule_file(f, &offset, sizeof(offset), -                              "Offset to capsule image")) +       if (write_capsule_file(f, &offsets, size * sizeof(uint64_t), +                              "Offsets to capsule images"))                  goto err;

-       /* -        * firmware capsule image header -        */ -       image.version = 0x00000003; -       memcpy(&image.update_image_type_id, guid, sizeof(*guid)); -       image.update_image_index = index; -       image.reserved[0] = 0; -       image.reserved[1] = 0; -       image.reserved[2] = 0; -       image.update_image_size = bin_size; -       if (auth_context.sig_size) -               image.update_image_size += sizeof(auth_context.auth) -                               + auth_context.sig_size; -       image.update_vendor_code_size = 0; /* none */ -       image.update_hardware_instance = instance; -       image.image_capsule_support = 0; -       if (auth_context.sig_size) -               image.image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION; -       if (write_capsule_file(f, &image, sizeof(image), -                              "Firmware capsule image header")) -               goto err;

-       /* -        * signature -        */ -       if (auth_context.sig_size) { -               if (write_capsule_file(f, &auth_context.auth,

• sizeof(auth_context.auth),

-                                      "Authentication header")) +       for (int i = 0; i < size; i++) { +               /* +                * firmware capsule image header +                */ +               images[i].version = 0x00000003;

memcpy(&images[i].update_image_type_id, &guids[i], sizeof(guids[i])); +               images[i].update_image_index = indices[i]; +               images[i].reserved[0] = 0; +               images[i].reserved[1] = 0; +               images[i].reserved[2] = 0; +               images[i].update_image_size = bin_sizes[i]; +               if (auth_contexts[i].sig_size)

images[i].update_image_size += sizeof(auth_contexts[i].auth) +                                       + auth_contexts[i].sig_size; +               images[i].update_vendor_code_size = 0; /* none */ +               images[i].update_hardware_instance = instances[i]; +               images[i].image_capsule_support = 0; +               if (auth_contexts[i].sig_size)

images[i].image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION; +               if (write_capsule_file(f, &images[i], sizeof(images[i]),

"Firmware capsule image header"))                          goto err;

-               if (write_capsule_file(f, auth_context.sig_data,

• auth_context.sig_size, "Signature"))

+               /* +                * signature +                */ +               if (auth_contexts[i].sig_size) { +                       if (write_capsule_file(f, &auth_contexts[i].auth,

• sizeof(auth_contexts[i].auth),
• "Authentication header"))

+                               goto err;

+                       if (write_capsule_file(f, auth_contexts[i].sig_data,

• auth_contexts[i].sig_size, "Signature"))

+                               goto err; +               }

+               /* +                * firmware binary +                */ +               if (write_capsule_file(f, buf_list[i], bin_sizes[i], "Firmware binary"))                          goto err;          }

-       /* -        * firmware binary -        */ -       if (write_capsule_file(f, buf, bin_size, "Firmware binary")) -               goto err;

ret = 0;   err:          if (f)                  fclose(f); -       free_sig_data(&auth_context); -       free(data); -       free(new_data); +       for (int i = 0; i < size; i++) { +               free_sig_data(&auth_contexts[i]); +               free(data_list[i]); +               free(new_data_list[i]); +       }

return ret;   } @@ -652,6 +676,228 @@ err:          return ret;   }

+/**

• • count_items - count number of items in list
• • @list:      Pointer to a string
• • @separator: Separator used to separate list items
• • Count the number of items in a list. The list items
• • are separated by a separator character inside the string.
• • Trailing white spaces are not allowed except if it is

the separator.

• • Return:
• • The item count.
• */

+int count_items(const char *list, char separator) +{ +       const char *c; +       int count = 0;

+       if (!*list) +               return 0;

+       for (c = list; *c; c++) { +               if (*c == separator) +                       count++; +       } +       /* correct count if no trailing separator present */ +       if (*(c - 1) != separator) +               count++;

+       return count; +}

+/**

• • update_itemcount - update item count
• • @count:     The count to be updated
• • @list:      The item list
• • @separator: List separator
• • Initialize the count if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with

the list count.

• • Return:
• • • 0 - on success
• • • -1 - if a check fails
• */

+int update_itemcount(int *count, const char *list, char separator) +{ +       int current_count = count_items(list, separator);

+       if (*count < 0) +               *count = current_count;

+       if (*count == 0 || +           *count != current_count) +               return -1;

+       return 0; +}

+/**

• • split_list - split list into elements
• • @elements:  Pointer to string array
• • @size:      The array size
• • @list:      The item list
• • @separator: List separator
• • Split a comma-separated list into its elements.
• • Return:
• • • 0 - on success
• • • -1 - on failure
• */

+int split_list(char **elements, int size, char *list, char separator) +{ +       const char separator_str[] = {separator, '\0'}; +       char *end;

+       for (int i = 0; i < size; i++) { +               elements[i] = strsep(&list, separator_str); +               if (!elements[i]) +                       return -1; +       }

+       end = strsep(&list, separator_str);  /* NULL or empty string expected */ +       if (end && *end) +               return -1;

+       return 0; +}

+/**

• • alloc_array - allocate memory for array
• • @count:     The number of elements
• • @obj_size:  The size of a single element
• • @name:      The name of the array
• • This is a wrapper for malloc which prints an error
• • message on failure.
• *  Return:
• • • Pointer to the allocated memory on success
• • • NULL on failure
• */

+void *alloc_array(unsigned int count, size_t obj_size, const char *name) +{ +       void *array;

+       array = malloc(count * obj_size); +       if (!array) +               fprintf(stderr, "Could not allocate memory for %s\n", name);

+       return array; +}

+/**

• • init_guids - populate guid array
• • @elements:  String array of elements to be converted
• • @size:      The array size
• • @name:      The name of the array
• • Allocate and populate an array of guid structs. The list

contains the UUIDs

• • to convert and store in the array. Upon failure an error

message is

• • printed.
• • Return:
• • • The initialized GUID array on success
• • • NULL on failure
• */

+efi_guid_t *init_guids(const char **elements, unsigned int size, +                      const char *name) +{ +       efi_guid_t *guids;

+       guids = alloc_array(size, sizeof(efi_guid_t), name); +       if (!guids) +               return NULL;

+       for (int i = 0; i < size; i++) { +               if (uuid_parse(elements[i], (unsigned char *)(guids + i))) { +                       fprintf(stderr, "Wrong %s format\n", name); +                       free(guids); +                       return NULL; +               } +               convert_uuid_to_guid((unsigned char *)(guids + i)); +       }

+       return guids; +}

+/**

• • init_uls - populate unsigned long array
• • @elements:  String array of elements to be converted
• • @size:      The array size
• • @name:      The name of the array
• • Allocate and populate an array of unsgined longs. Upon failure an
• • error message is printed.
• • Return:
• • • The initialized array on success
• • • NULL on failure
• */

+unsigned long *init_uls(const char **elements, unsigned int size, +                       const char *name) +{ +       unsigned long *array;

+       array = alloc_array(size, sizeof(unsigned long), name); +       if (!array) +               return NULL; +       for (int i = 0; i < size; i++) +               array[i] = strtoul(elements[i], NULL, 0);

+       return array; +}

+/**

• • init_list - parse list and allocate elements
• • @listcount: The list count to be checked and updated
• • @list:      The list to be parsed
• • @separator: The list separator
• • @name:      The name of the list
• • @multiple_times:    List encountered multiple times
• • Routine for command line argument lists.
• • Parse the string list and count the list elements.
• • Initialize the listcount if it is uninitialized (negative value).
• • Check that the list contains at least one item.
• • Check if an already initialized count is consistent with

the list count.

• • Allocate the string array and populate it with the list elements.
• • The array should be freed in the calling function.
• • Upon failure an error message is printed and the program exits.
• *  Return:
• • • The initialized array on success
• • • NULL on failure
• */

+char **init_list(int *listcount, char *list, char separator, +                bool multiple_times, char *name) +{ +       char **elements;

+       if (multiple_times) { +               fprintf(stderr, "%s specified multiple times\n", name); +               return NULL; +       } +       if (update_itemcount(listcount, list, separator)) { +               fprintf(stderr, "List count not consistent with previous or list not provided\n"); +               return NULL; +       } +       elements = alloc_array(*listcount, sizeof(char *), name); +       if (!elements) +               return NULL; +       if (split_list(elements, *listcount, list, separator)) { +               fprintf(stderr, "Could not parse %s list\n", name); +               free(elements); +               return NULL; +       }

+       return elements; +}

/**    * main - main entry function of mkeficapsule    * @argc:      Number of arguments @@ -666,24 +912,27 @@ err:    */   int main(int argc, char **argv)   { -       efi_guid_t *guid; -       unsigned char uuid_buf[16]; -       unsigned long index, instance; -       uint64_t mcount; +       const char separator = ','; +       const efi_guid_t *guids; /* an array */ +       const unsigned long *indices, *instances, *mcounts, *fw_versions; /* arrays */          unsigned long oemflags; +       const char **blob_paths, **elements;  /* string arrays */          const char *privkey_file, *cert_file; -       int c, idx; -       struct fmp_payload_header_params fmp_ph_params = { 0 }; +       int listcount, c, idx;

-       guid = NULL; -       index = 0; -       instance = 0; -       mcount = 0; +       guids = NULL; +       indices = NULL; +       instances = NULL; +       mcounts = NULL; +       oemflags = 0; +       blob_paths = NULL;          privkey_file = NULL;          cert_file = NULL; +       elements = NULL; +       listcount = -1; +       fw_versions = NULL;          dump_sig = 0;          capsule_type = CAPSULE_NORMAL_BLOB; -       oemflags = 0;          for (;;) {                  c = getopt_long(argc, argv, opts_short, options, &idx);                  if (c == -1) @@ -691,27 +940,62 @@ int main(int argc, char **argv)

switch (c) {                  case 'g': -                       if (guid) { -                               fprintf(stderr,

"Image type already specified\n"); +                       elements = (const char **)init_list(&listcount, optarg, separator, !!guids,

• "GUID");

+                       if (!elements)                                  exit(EXIT_FAILURE); -                       } -                       if (uuid_parse(optarg, uuid_buf)) {

fprintf(stderr, "Wrong guid format\n");

+                       guids = init_guids(elements, listcount, "GUID"); +                       if (!guids)                                  exit(EXIT_FAILURE); -                       } -                       convert_uuid_to_guid(uuid_buf); -                       guid = (efi_guid_t *)uuid_buf;

+                       free(elements); +                       elements = NULL;                          break;                  case 'i': -                       index = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!indices, "index");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       indices = init_uls(elements, listcount, "index"); +                       if (!indices) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL; +                       break; +               case 'b': +                       blob_paths = (const char **)init_list(&listcount, optarg, separator,

• !!blob_paths, "blob path");

+                       if (!blob_paths) +                               exit(EXIT_FAILURE);                          break;                  case 'I': -                       instance = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!instances, "instance");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       instances = init_uls(elements, listcount, "instance"); +                       if (!instances) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'v':

fmp_ph_params.fw_version = strtoul(optarg, NULL, 0); -                       fmp_ph_params.have_header = true; +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!fw_versions, "firmware version");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       fw_versions = init_uls(elements, listcount, "firmware version"); +                       if (!fw_versions) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'p':                          if (privkey_file) { @@ -730,7 +1014,17 @@ int main(int argc, char **argv)                          cert_file = optarg;                          break;                  case 'm': -                       mcount = strtoul(optarg, NULL, 0); +                       elements = (const char **)init_list(&listcount, optarg, separator,

• !!mcounts, "monotonic count");

+                       if (!elements) +                               exit(EXIT_FAILURE);

+                       mcounts = init_uls(elements, listcount, "monotonic count"); +                       if (!mcounts) +                               exit(EXIT_FAILURE);

+                       free(elements); +                       elements = NULL;                          break;                  case 'd':                          dump_sig = 1; @@ -767,26 +1061,46 @@ int main(int argc, char **argv)

/* check necessary parameters */          if ((capsule_type == CAPSULE_NORMAL_BLOB && -           ((argc != optind + 2) || !guid || -            ((privkey_file && !cert_file) || -             (!privkey_file && cert_file)))) || +            (!((argc != optind + 2) ^ !(blob_paths && argc == optind + 1)) || !guids || +             ((privkey_file && !cert_file) || +              (!privkey_file && cert_file)))) ||              (capsule_type != CAPSULE_NORMAL_BLOB && -           ((argc != optind + 1) || -            ((capsule_type == CAPSULE_ACCEPT) && !guid) || -            ((capsule_type == CAPSULE_REVERT) && guid)))) { +            ((argc != optind + 1) || +             ((capsule_type == CAPSULE_ACCEPT) && !guids) || +             ((capsule_type == CAPSULE_ACCEPT) && listcount != 1) || +             ((capsule_type == CAPSULE_REVERT) && guids)))) {                  print_usage();                  exit(EXIT_FAILURE);          }

+       /* populate blob_paths if image blob was provided as positional argument */ +       if (capsule_type == CAPSULE_NORMAL_BLOB && !blob_paths) { +               blob_paths = malloc(sizeof(char *)); +               if (!blob_paths) {

fprintf(stderr, "Could not allocate memory for blob paths\n"); +                       exit(EXIT_FAILURE); +               } +               *blob_paths = argv[argc - 2]; +       }

+       /* populate arrays with zeros if they are not provided */ +       if (!indices) +               indices = calloc(listcount, sizeof(unsigned long)); +       if (!instances) +               instances = calloc(listcount, sizeof(unsigned long)); +       if (!mcounts) +               mcounts = calloc(listcount, sizeof(uint64_t));

if (capsule_type != CAPSULE_NORMAL_BLOB) { -               if (create_empty_capsule(argv[argc - 1], guid, +               if (create_empty_capsule(argv[argc - 1], guids,

capsule_type == CAPSULE_ACCEPT) < 0) {

fprintf(stderr, "Creating empty capsule failed\n");                          exit(EXIT_FAILURE);                  } -       } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,

index, instance, &fmp_ph_params, mcount, privkey_file, -                                cert_file, (uint16_t)oemflags) < 0) { +       } else if (create_fwbin(argv[argc - 1], blob_paths, guids, +                               indices, instances, fw_versions, +                               mcounts, listcount, privkey_file, +                               cert_file, (uint16_t)oemflags) < 0) {                  fprintf(stderr, "Creating firmware capsule failed\n");                  exit(EXIT_FAILURE);          } -- 2.30.2