[PATCH v2 2/3] Revert "mkeficapsule: Remove dtb related options"

Simon Glass sjg at chromium.org
Wed Aug 4 18:08:12 CEST 2021


Hi Takahiro,

On Mon, 2 Aug 2021 at 23:30, KASHI Takahiro <takahiro.akashi at linaro.org> wrote:
>
> Simon,
>
> On Mon, Aug 02, 2021 at 08:44:30AM -0600, Simon Glass wrote:
> > This reverts commit f86caab058ff062ce72b24cd1ab9ec1253cc1352.
>
> Whether we choose a devicetree approach or not, we don't have
> to revert this patch because we can do the same thing with
> standard commands (i.e. fdtoverlay).
>
> See my patches and discussions[1].
> (We all agreed to removing this feature from mkeficapsule.)

Well this just shows that I am not the right person to be sorting out
this mess. Can someone in the team working on come up with the right
fix?

Regards,
Simon

>
> [1] https://lists.denx.de/pipermail/u-boot/2021-May/449573.html
>     https://lists.denx.de/pipermail/u-boot/2021-May/449574.html
>
> -Takahiro Akashi
>
>
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  tools/mkeficapsule.c | 229 +++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 222 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index 4995ba4e0c2..de0a6289888 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -4,17 +4,22 @@
> >   *           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>
> >  #include <sys/stat.h>
> >  #include <sys/types.h>
> >
> > +#include "fdt_host.h"
> > +
> >  typedef __u8 u8;
> >  typedef __u16 u16;
> >  typedef __u32 u32;
> > @@ -24,6 +29,9 @@ typedef __s32 s32;
> >
> >  #define aligned_u64 __aligned_u64
> >
> > +#define SIGNATURE_NODENAME   "signature"
> > +#define OVERLAY_NODENAME     "__overlay__"
> > +
> >  #ifndef __packed
> >  #define __packed __attribute__((packed))
> >  #endif
> > @@ -44,6 +52,9 @@ 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},
> >  };
> > @@ -57,10 +68,187 @@ 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)
> >  {
> > @@ -178,16 +366,22 @@ 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:h", options, &idx);
> > +             c = getopt_long(argc, argv, "f:r:i:I:v:D:K:Oh", options, &idx);
> >               if (c == -1)
> >                       break;
> >
> > @@ -214,22 +408,43 @@ 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 an output file */
> > -     if (argc != optind + 1) {
> > +     /* need a fit image file or raw image file */
> > +     if (!file && !pkey_file && !dtb_file) {
> >               print_usage();
> >               exit(EXIT_FAILURE);
> >       }
> >
> > -     /* need a fit image file or raw image file */
> > -     if (!file) {
> > -             print_usage();
> > -             exit(EXIT_SUCCESS);
> > +     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);
> > +             }
> >       }
> >
> >       if (create_fwbin(argv[optind], file, guid, index, instance)
> > --
> > 2.32.0.554.ge1b32706d8-goog
> >


More information about the U-Boot mailing list