From: Takahiro Akashi <takahiro.akashi@linaro.org>
To: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Alexander Graf <agraf@csgraf.de>,
Sughosh Ganu <sughosh.ganu@linaro.org>,
Simon Glass <sjg@chromium.org>,
U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH 2/3] mkeficapsule: Remove dtb related options
Date: Fri, 16 Jul 2021 22:53:37 +0900 [thread overview]
Message-ID: <20210716135337.GC68948@laputa> (raw)
In-Reply-To: <CAA93ih2KyTYHTP-Vn_b=c-VPFiGnNc5i5dKTKrSuJruZ4w+vSg@mail.gmail.com>
On Fri, Jul 16, 2021 at 02:57:54PM +0900, Masami Hiramatsu wrote:
> 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>
I didn't try the compilation, but I don't think
we need neither <sys/mman.h> nor "fdt_host.h".
-Takahiro Akashi
> > @@ -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
next prev parent reply other threads:[~2021-07-16 13:54 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-15 17:00 [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata Ilias Apalodimas
2021-07-15 17:00 ` [PATCH 2/3] mkeficapsule: Remove dtb related options Ilias Apalodimas
2021-07-16 5:57 ` Masami Hiramatsu
2021-07-16 13:53 ` Takahiro Akashi [this message]
2021-07-16 14:03 ` Simon Glass
2021-07-17 7:24 ` Ilias Apalodimas
2021-07-20 18:33 ` Simon Glass
2021-07-20 18:43 ` Ilias Apalodimas
2021-07-20 18:50 ` Simon Glass
2021-07-15 17:00 ` [PATCH 3/3] doc: Update CapsuleUpdate READMEs Ilias Apalodimas
2021-07-16 6:50 ` Heinrich Schuchardt
2021-07-16 7:09 ` Ilias Apalodimas
2021-07-16 12:33 ` AKASHI Takahiro
2021-07-16 5:57 ` [PATCH 1/3] efi_capsule: Move signature from DTB to .rodata Masami Hiramatsu
2021-07-16 13:39 ` Takahiro Akashi
2021-07-17 11:35 ` Ilias Apalodimas
2021-07-17 14:14 ` Ilias Apalodimas
2021-07-16 6:44 ` Sughosh Ganu
2021-07-16 13:49 ` Simon Glass
2021-07-17 11:36 ` Ilias Apalodimas
-- strict thread matches above, loose matches on Subject: below --
2021-07-17 14:26 [PATCH 1/3 v2] " Ilias Apalodimas
2021-07-17 14:26 ` [PATCH 2/3] mkeficapsule: Remove dtb related options Ilias Apalodimas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210716135337.GC68948@laputa \
--to=takahiro.akashi@linaro.org \
--cc=agraf@csgraf.de \
--cc=ilias.apalodimas@linaro.org \
--cc=masami.hiramatsu@linaro.org \
--cc=sjg@chromium.org \
--cc=sughosh.ganu@linaro.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox