Re: [PATCH 2/3] mkeficapsule: Remove dtb related options

[Masami Hiramatsu <masami.hiramatsu@linaro.org>]

2021年7月16日(金) 2:00 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>
> commit 322c813f4bec ("mkeficapsule: Add support for embedding public key in a dtb")
> added a bunch of options enabling the addition of the capsule public key
> in a dtb.  Since now we embeded the key in U-Boot's .rodata we don't this
> this functionality anymore

This looks good to me.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Thanks,

>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  tools/mkeficapsule.c | 226 ++-----------------------------------------
>  1 file changed, 7 insertions(+), 219 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index de0a62898886..214dc38e46e3 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -4,14 +4,12 @@
>   *             Author: AKASHI Takahiro
>   */
>
> -#include <errno.h>
>  #include <getopt.h>
>  #include <malloc.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> -#include <unistd.h>
>  #include <linux/types.h>
>
>  #include <sys/mman.h>
> @@ -29,9 +27,6 @@ typedef __s32 s32;
>
>  #define aligned_u64 __aligned_u64
>
> -#define SIGNATURE_NODENAME     "signature"
> -#define OVERLAY_NODENAME       "__overlay__"
> -
>  #ifndef __packed
>  #define __packed __attribute__((packed))
>  #endif
> @@ -52,9 +47,6 @@ static struct option options[] = {
>         {"raw", required_argument, NULL, 'r'},
>         {"index", required_argument, NULL, 'i'},
>         {"instance", required_argument, NULL, 'I'},
> -       {"dtb", required_argument, NULL, 'D'},
> -       {"public key", required_argument, NULL, 'K'},
> -       {"overlay", no_argument, NULL, 'O'},
>         {"help", no_argument, NULL, 'h'},
>         {NULL, 0, NULL, 0},
>  };
> @@ -68,187 +60,10 @@ static void print_usage(void)
>                "\t-r, --raw <raw image>       new raw image file\n"
>                "\t-i, --index <index>         update image index\n"
>                "\t-I, --instance <instance>   update hardware instance\n"
> -              "\t-K, --public-key <key file> public key esl file\n"
> -              "\t-D, --dtb <dtb file>        dtb file\n"
> -              "\t-O, --overlay               the dtb file is an overlay\n"
>                "\t-h, --help                  print a help message\n",
>                tool_name);
>  }
>
> -static int fdt_add_pub_key_data(void *sptr, void *dptr, size_t key_size,
> -                               bool overlay)
> -{
> -       int parent;
> -       int ov_node;
> -       int frag_node;
> -       int ret = 0;
> -
> -       if (overlay) {
> -               /*
> -                * The signature would be stored in the
> -                * first fragment node of the overlay
> -                */
> -               frag_node = fdt_first_subnode(dptr, 0);
> -               if (frag_node == -FDT_ERR_NOTFOUND) {
> -                       fprintf(stderr,
> -                               "Couldn't find the fragment node: %s\n",
> -                               fdt_strerror(frag_node));
> -                       goto done;
> -               }
> -
> -               ov_node = fdt_subnode_offset(dptr, frag_node, OVERLAY_NODENAME);
> -               if (ov_node == -FDT_ERR_NOTFOUND) {
> -                       fprintf(stderr,
> -                               "Couldn't find the __overlay__ node: %s\n",
> -                               fdt_strerror(ov_node));
> -                       goto done;
> -               }
> -       } else {
> -               ov_node = 0;
> -       }
> -
> -       parent = fdt_subnode_offset(dptr, ov_node, SIGNATURE_NODENAME);
> -       if (parent == -FDT_ERR_NOTFOUND) {
> -               parent = fdt_add_subnode(dptr, ov_node, SIGNATURE_NODENAME);
> -               if (parent < 0) {
> -                       ret = parent;
> -                       if (ret != -FDT_ERR_NOSPACE) {
> -                               fprintf(stderr,
> -                                       "Couldn't create signature node: %s\n",
> -                                       fdt_strerror(parent));
> -                       }
> -               }
> -       }
> -       if (ret)
> -               goto done;
> -
> -       /* Write the key to the FDT node */
> -       ret = fdt_setprop(dptr, parent, "capsule-key",
> -                         sptr, key_size);
> -
> -done:
> -       if (ret)
> -               ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> -
> -       return ret;
> -}
> -
> -static int add_public_key(const char *pkey_file, const char *dtb_file,
> -                         bool overlay)
> -{
> -       int ret;
> -       int srcfd = -1;
> -       int destfd = -1;
> -       void *sptr = NULL;
> -       void *dptr = NULL;
> -       off_t src_size;
> -       struct stat pub_key;
> -       struct stat dtb;
> -
> -       /* Find out the size of the public key */
> -       srcfd = open(pkey_file, O_RDONLY);
> -       if (srcfd == -1) {
> -               fprintf(stderr, "%s: Can't open %s: %s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fstat(srcfd, &pub_key);
> -       if (ret == -1) {
> -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       src_size = pub_key.st_size;
> -
> -       /* mmap the public key esl file */
> -       sptr = mmap(0, src_size, PROT_READ, MAP_SHARED, srcfd, 0);
> -       if (sptr == MAP_FAILED) {
> -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> -                       __func__, pkey_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       /* Open the dest FDT */
> -       destfd = open(dtb_file, O_RDWR);
> -       if (destfd == -1) {
> -               fprintf(stderr, "%s: Can't open %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fstat(destfd, &dtb);
> -       if (ret == -1) {
> -               fprintf(stderr, "%s: Can't stat %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               goto err;
> -       }
> -
> -       dtb.st_size += src_size + 0x30;
> -       if (ftruncate(destfd, dtb.st_size)) {
> -               fprintf(stderr, "%s: Can't expand %s: %s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       errno = 0;
> -       /* mmap the dtb file */
> -       dptr = mmap(0, dtb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED,
> -                   destfd, 0);
> -       if (dptr == MAP_FAILED) {
> -               fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> -                       __func__, dtb_file, strerror(errno));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       if (fdt_check_header(dptr)) {
> -               fprintf(stderr, "%s: Invalid FDT header\n", __func__);
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = fdt_open_into(dptr, dptr, dtb.st_size);
> -       if (ret) {
> -               fprintf(stderr, "%s: Cannot expand FDT: %s\n",
> -                       __func__, fdt_strerror(ret));
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       /* Copy the esl file to the expanded FDT */
> -       ret = fdt_add_pub_key_data(sptr, dptr, src_size, overlay);
> -       if (ret < 0) {
> -               fprintf(stderr, "%s: Unable to add public key to the FDT\n",
> -                       __func__);
> -               ret = -1;
> -               goto err;
> -       }
> -
> -       ret = 0;
> -
> -err:
> -       if (sptr)
> -               munmap(sptr, src_size);
> -
> -       if (dptr)
> -               munmap(dptr, dtb.st_size);
> -
> -       if (srcfd != -1)
> -               close(srcfd);
> -
> -       if (destfd != -1)
> -               close(destfd);
> -
> -       return ret;
> -}
> -
>  static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>                         unsigned long index, unsigned long instance)
>  {
> @@ -366,22 +181,16 @@ err_1:
>  int main(int argc, char **argv)
>  {
>         char *file;
> -       char *pkey_file;
> -       char *dtb_file;
>         efi_guid_t *guid;
>         unsigned long index, instance;
>         int c, idx;
> -       int ret;
> -       bool overlay = false;
>
>         file = NULL;
> -       pkey_file = NULL;
> -       dtb_file = NULL;
>         guid = NULL;
>         index = 0;
>         instance = 0;
>         for (;;) {
> -               c = getopt_long(argc, argv, "f:r:i:I:v:D:K:Oh", options, &idx);
> +               c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
>                 if (c == -1)
>                         break;
>
> @@ -408,43 +217,22 @@ int main(int argc, char **argv)
>                 case 'I':
>                         instance = strtoul(optarg, NULL, 0);
>                         break;
> -               case 'K':
> -                       if (pkey_file) {
> -                               printf("Public Key already specified\n");
> -                               return -1;
> -                       }
> -                       pkey_file = optarg;
> -                       break;
> -               case 'D':
> -                       if (dtb_file) {
> -                               printf("DTB file already specified\n");
> -                               return -1;
> -                       }
> -                       dtb_file = optarg;
> -                       break;
> -               case 'O':
> -                       overlay = true;
> -                       break;
>                 case 'h':
>                         print_usage();
>                         return 0;
>                 }
>         }
>
> -       /* need a fit image file or raw image file */
> -       if (!file && !pkey_file && !dtb_file) {
> +       /* need a output file */
> +       if (argc != optind + 1) {
>                 print_usage();
>                 exit(EXIT_FAILURE);
>         }
>
> -       if (pkey_file && dtb_file) {
> -               ret = add_public_key(pkey_file, dtb_file, overlay);
> -               if (ret == -1) {
> -                       printf("Adding public key to the dtb failed\n");
> -                       exit(EXIT_FAILURE);
> -               } else {
> -                       exit(EXIT_SUCCESS);
> -               }
> +       /* need a fit image file or raw image file */
> +       if (!file) {
> +               print_usage();
> +               exit(EXIT_SUCCESS);
>         }
>
>         if (create_fwbin(argv[optind], file, guid, index, instance)
> --
> 2.32.0.rc0
>


-- 
Masami Hiramatsu