Re: [PATCH v7 01/12] tools: mkeficapsule: rework the code a little bit

On 11/16/21 05:32, AKASHI Takahiro wrote:

Abstract common routines to make the code easily understandable. No functional change.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org Reviewed-by: Simon Glass sjg@chromium.org

---

tools/mkeficapsule.c | 223 ++++++++++++++++++++++++++++++------------- 1 file changed, 159 insertions(+), 64 deletions(-)

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index 4995ba4e0c2a..afdcaf7e7933 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -61,17 +61,122 @@ static void print_usage(void) tool_name); }

+/**

• • read_bin_file - read a firmware binary file
• • @bin: Path to a firmware binary file
• • @data: Pointer to pointer of allocated buffer
• • @bin_size: Size of allocated buffer
• • Read out a content of binary, @bin, into @data.
• • A caller should free @data.
• • Return:
• • • 0 - on success
• • • -1 - on failure
• */

+static int read_bin_file(char *bin, void **data, off_t *bin_size) +{

• FILE *g;
• struct stat bin_stat;
• void *buf;
• size_t size;
• int ret = 0;
• g = fopen(bin, "r");
• if (!g) {

• printf("cannot open %s\n", bin);
  

Error messages should be written to stderr. Use perror(bin) to get a message like:

<filename>: No such file or directory

Or use fprintf(stderr, ...).

• return -1;
  

• }
• if (stat(bin, &bin_stat) < 0) {

• printf("cannot determine the size of %s\n", bin);
  

• ret = -1;
  

• goto err;
  

• }
• if (bin_stat.st_size > (u32)~0U) {

• printf("file size is too large: %s\n", bin);
  

• ret = -1;
  

• goto err;
  

• }
• buf = malloc(bin_stat.st_size);

For a symbolic link st_size is the length of the pathname it contains. So this function is of no help here.

• if (!buf) {

• printf("cannot allocate memory: %zx\n",
  

•        (size_t)bin_stat.st_size);
  

• ret = -1;
  

• goto err;
  

• }
• size = fread(buf, 1, bin_stat.st_size, g);
• if (size < bin_stat.st_size) {

fread() reads from a stream which is not necessarily a file but can be a named pipe. stat() does not indicate when the stream will end.

You have to run fread() in a loop until feof() or ferror() indicate that there is nothing more to be read from the stream.

Best regards

Heinrich

• printf("read failed (%zx)\n", size);
  

• ret = -1;
  

• goto err;
  

• }
• *data = buf;
• *bin_size = bin_stat.st_size;

+err:

• fclose(g);
• return ret;

+}

+/**

• • write_capsule_file - write a capsule file
• • @bin: FILE stream
• • @data: Pointer to data
• • @bin_size: Size of data
• • Write out data, @data, with the size @bin_size.
• • Return:
• • • 0 - on success
• • • -1 - on failure
• */

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

• size_t size_written;
• size_written = fwrite(data, 1, size, f);
• if (size_written < size) {

• printf("%s: write failed (%zx != %zx)\n", msg,
  

•        size_written, size);
  

• return -1;
  

• }
• return 0;

+}

+/**

• • 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
• • @instance: Instance number in capsule
• • @mcount: Monotonic count in authentication information
• • @private_file: Path to a private key file
• • @cert_file: Path to a certificate file
• • This function actually does the job of creating an uefi capsule file.
• • All the arguments must be supplied.
• • If either @private_file ror @cert_file is NULL, the capsule file
• • won't be signed.
• • Return:
• • • 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 efi_capsule_header header; struct efi_firmware_management_capsule_header capsule; struct efi_firmware_management_capsule_image_header image;

• FILE *f, *g;
• struct stat bin_stat;
• u8 *data;
• size_t size;

• FILE *f;

• void *data;

• off_t bin_size; u64 offset;

• int ret;

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

@@ -79,25 +184,28 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, printf("\tindex: %ld\n\tinstance: %ld\n", index, instance); #endif

• g = fopen(bin, "r");
• if (!g) {

• printf("cannot open %s\n", bin);
  

• return -1;
  

• }
• if (stat(bin, &bin_stat) < 0) {

• printf("cannot determine the size of %s\n", bin);
  

• goto err_1;
  

• }
• data = malloc(bin_stat.st_size);
• if (!data) {

• printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
  

• goto err_1;
  

• }

• f = NULL;
• data = NULL;
• ret = -1;
• /*

• * read a firmware binary
  

• */
  

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

• goto err;
  

• /*

• * write a capsule file
  

• */
  

  f = fopen(path, "w"); if (!f) { printf("cannot open %s\n", path);

• goto err_2;
  

• goto err;
  

  }
• /*

• * capsule file header
  

• */
  

  header.capsule_guid = efi_guid_fm_capsule; header.header_size = sizeof(header); /* TODO: The current implementation ignores flags */

@@ -105,70 +213,57 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid, header.capsule_image_size = sizeof(header) + sizeof(capsule) + sizeof(u64) + sizeof(image)

• 			+ bin_stat.st_size;
  

• size = fwrite(&header, 1, sizeof(header), f);
• if (size < sizeof(header)) {

• printf("write failed (%zx)\n", size);
  

• goto err_3;
  

• }

• 			+ bin_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;

• size = fwrite(&capsule, 1, sizeof(capsule), f);
• if (size < (sizeof(capsule))) {

• printf("write failed (%zx)\n", size);
  

• goto err_3;
  

• }

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

• 	       "Firmware capsule header"))
  

• goto err;
  

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

• size = fwrite(&offset, 1, sizeof(offset), f);
• if (size < sizeof(offset)) {

• printf("write failed (%zx)\n", size);
  

• goto err_3;
  

• }

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

• 	       "Offset to capsule image"))
  

• 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_stat.st_size;

• image.update_image_size = bin_size; image.update_vendor_code_size = 0; /* none */ image.update_hardware_instance = instance; image.image_capsule_support = 0;
• if (write_capsule_file(f, &image, sizeof(image),

• 	       "Firmware capsule image header"))
  

• goto err;
  
  

• size = fwrite(&image, 1, sizeof(image), f);
• if (size < sizeof(image)) {

• printf("write failed (%zx)\n", size);
  

• goto err_3;
  

• }
• size = fread(data, 1, bin_stat.st_size, g);
• if (size < bin_stat.st_size) {

• printf("read failed (%zx)\n", size);
  

• goto err_3;
  

• }
• size = fwrite(data, 1, bin_stat.st_size, f);
• if (size < bin_stat.st_size) {

• printf("write failed (%zx)\n", size);
  

• goto err_3;
  

• }
• fclose(f);
• fclose(g);
• free(data);
• return 0;

• /*

• * firmware binary
  

• */
  

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

• goto err;
  
  

-err_3:

• fclose(f);

-err_2:

• ret = 0;

+err:

• if (f)

• fclose(f);
  

  free(data);

-err_1:

• fclose(g);

• return -1;

• return ret; }

  /*