From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 72EB8C433F5 for ; Thu, 6 Jan 2022 09:01:47 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 08272830D9; Thu, 6 Jan 2022 10:01:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="KN+7mpvQ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D82CA830DA; Thu, 6 Jan 2022 10:01:42 +0100 (CET) Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D162B830CE for ; Thu, 6 Jan 2022 10:01:38 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pg1-x531.google.com with SMTP id i8so2020793pgt.13 for ; Thu, 06 Jan 2022 01:01:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=nEfTejBFItKEPxYxcQoqgiAvSvqCbk6dgAlGUj7NdGw=; b=KN+7mpvQgkOvcWthxjCJ/CWOhWAhUqQZ0FIhZt7ECeb9v/4yiJOj+AW6S408Ng/OqD QUgE4+ES27zDJ+QtRLQ/vObVHJZNl6xKy4cxaZpuBLDUODRhLRLoD31sqrxJYsFPTjxF NuHkxdOIvkYps3rjTFKVtXTRFQ7bLuQpqo2JPGLd7Yjc9L0cgqrhq/oygSN9MPdfJrIj ozH5ybzqC11eGqcfGtXiBOMv6DylIVt5unrG22iGzHftpPxjBuu594fU+SobsMmpxcPo TJcIAC3BQkma80cK2+ygvRSRPaPRtCQnzSL5zTkyjHEQoVtPlvcl+Ceg7ir2IgYiomy5 XO/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=nEfTejBFItKEPxYxcQoqgiAvSvqCbk6dgAlGUj7NdGw=; b=Y1SX7R1vihpC8cjhoJBQRkpQmmYKlQSW2crPYFyfYm/ae9PYfljiNAuSZ+mioG1Wxd JVL9XYuoiM8OUAqIhOU792iGubETRS79MxxZnI0OZqYLtf48FZBzbOP0cgZg6zDS6uRl grx7lkqr5MO8tNS4w4VzqEBNRe7rAyeQ//1uyFlX2/l/I8beZaFsjHGUde+Lb4VBeI7j f1zhM3h0d7BiSwMik8QnbiuvWfXeuiwl2kQldMmla8AyMWivDk9bB4GCTDHz2a7FypwB rFJ8Yfy444tAiSTeTD2HjUr5cTNIrY+qcc9DPEMQCeYJ6dCt5CHruXoZcVoOSKLSM61P TNoQ== X-Gm-Message-State: AOAM531fJXW3mVppEAZy+CBgSU2UaJrZSLYmfLkwZrudP3Chy9OQU+5Q ot9bjA43BYcITJAFuj72ccipIA== X-Google-Smtp-Source: ABdhPJwO8IlSOoDvO/GIQ4CRgINAgf1dO9Qevk4yUi3FbuNo+uD1DNFwPOZGI53widzMncie+k1J/A== X-Received: by 2002:a05:6a00:22c6:b0:4bb:b34b:3c30 with SMTP id f6-20020a056a0022c600b004bbb34b3c30mr52660472pfj.70.1641459696769; Thu, 06 Jan 2022 01:01:36 -0800 (PST) Received: from laputa ([2400:4050:c3e1:100:9127:7a05:487c:d781]) by smtp.gmail.com with ESMTPSA id c7sm5043425pjs.17.2022.01.06.01.01.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jan 2022 01:01:36 -0800 (PST) Date: Thu, 6 Jan 2022 18:01:31 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: agraf@csgraf.de, sjg@chromium.org, ilias.apalodimas@linaro.org, sughosh.ganu@linaro.org, masami.hiramatsu@linaro.org, mark.kettenis@xs4all.nl, u-boot@lists.denx.de Subject: Re: [PATCH v8 01/12] tools: mkeficapsule: rework the code a little bit Message-ID: <20220106090131.GF45004@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , agraf@csgraf.de, sjg@chromium.org, ilias.apalodimas@linaro.org, sughosh.ganu@linaro.org, masami.hiramatsu@linaro.org, mark.kettenis@xs4all.nl, u-boot@lists.denx.de References: <20211220050253.31163-1-takahiro.akashi@linaro.org> <20211220050253.31163-2-takahiro.akashi@linaro.org> <448e9927-2fd6-bff2-5a1b-45308b57e007@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <448e9927-2fd6-bff2-5a1b-45308b57e007@gmx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Sat, Jan 01, 2022 at 10:35:04PM +0100, Heinrich Schuchardt wrote: > On 12/20/21 06:02, AKASHI Takahiro wrote: > > Abstract common routines to make the code easily understandable. > > No functional change. > > > > Signed-off-by: AKASHI Takahiro > > Reviewed-by: Simon Glass > > --- > > 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 output should be to stderr. Ah, I've forgot to fix it. > > + return -1; > > + } > > + if (stat(bin, &bin_stat) < 0) { > > + printf("cannot determine the size of %s\n", bin); > > Error output should be to stderr. > > > + ret = -1; > > + goto err; > > + } > > + if (bin_stat.st_size > (u32)~0U) { > > malloc() expects a size_t argument. Why don't you compare to SIZE_MAX > defined in stdint.h here? OK. -Takahiro Akashi > > + printf("file size is too large: %s\n", bin); > > Error output should be to stderr. > > > + ret = -1; > > + goto err; > > + } > > + buf = malloc(bin_stat.st_size); > > + if (!buf) { > > + printf("cannot allocate memory: %zx\n", > > Error output should be to stderr. > > > + (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) { > > + printf("read failed (%zx)\n", size); > > Error output should be to stderr. > > > + 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, > > Error output should be to stderr. > > > + 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); > > Error output should be to stderr. > > Best regards > > Heinrich > > > - 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; > > } > > > > /* >