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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8B21C433EF for ; Fri, 1 Oct 2021 06:23:56 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 7069861A50 for ; Fri, 1 Oct 2021 06:23:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 7069861A50 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id DC469820ED; Fri, 1 Oct 2021 08:23:54 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1633069435; bh=N5iehps0OAG8KThwfcT0rQ3rBgjk9PboohRhRBWHe20=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=c8QGeOsXrHOjw0tG467qU9TTeJ0gLFulZyyIR8cOK+oO7vMi9MLBJDPCDw1mqVoA1 McHxb6hdGEyjEDvMZ6OHp6gqWmNxrrzBXn4fdY2l/5crp5WqC/3VD3sVx5+9TehxVa CEwdO+3n5hJM6fbPRLan2Iw1t/BExCaQb25tgPasITtsA3/jvc/ZBwr2va8JyKPDwv zyHgyaeec3WkcdpktzVddaZMy5vmiACN7R0cnn42/AIqEOkwZ5Kkc4DAVs1lzWnwuo DBYz5W5SI4iOSh9yvwg61uUjFwRtkl3X4tVCLDM8j2AN+DG/0K0g8Bb79zWj5PRRZt RvT7mj6sZ4YfA== Received: by phobos.denx.de (Postfix, from userid 109) id AAE1D82C6D; Fri, 1 Oct 2021 08:23:53 +0200 (CEST) Received: from mout-u-204.mailbox.org (mout-u-204.mailbox.org [91.198.250.253]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D5C82820E3 for ; Fri, 1 Oct 2021 08:23:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp202.mailbox.org (smtp202.mailbox.org [80.241.60.245]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-204.mailbox.org (Postfix) with ESMTPS id 4HLKnK5frBzQjcJ; Fri, 1 Oct 2021 08:23:49 +0200 (CEST) Subject: Re: [PATCH u-boot-marvell v3 24/39] tools: kwbimage: Refactor kwbimage header size determination To: =?UTF-8?Q?Marek_Beh=c3=ban?= Cc: u-boot@lists.denx.de, pali@kernel.org, Chris Packham , Baruch Siach , Dennis Gilmore , Mario Six , Jon Nettleton , =?UTF-8?Q?Marek_Beh=c3=ban?= References: <20210924210716.29752-1-kabel@kernel.org> <20210924210716.29752-25-kabel@kernel.org> From: Stefan Roese Message-ID: <45d30832-8261-b021-a54f-cf9c93b56e89@denx.de> Date: Fri, 1 Oct 2021 08:23:45 +0200 MIME-Version: 1.0 In-Reply-To: <20210924210716.29752-25-kabel@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 78D79269 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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 24.09.21 23:07, Marek Behún wrote: > From: Marek Behún > > Add functions kwbheader_size() and kwbheader_size_for_csum(). > > Refactor code determining header size to use these functions. > > Refactor header checksum determining function. > > Remove stuff that is not needed anymore. > > This simplifies the code a little and fixes one instance of validating > header size meant for checksum instead of whole header size. > > Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan > --- > tools/kwbimage.c | 29 +++++++---------------------- > tools/kwbimage.h | 35 +++++++++++++++++++++++------------ > tools/kwboot.c | 22 ++++++++++------------ > 3 files changed, 40 insertions(+), 46 deletions(-) > > diff --git a/tools/kwbimage.c b/tools/kwbimage.c > index 944a108cee..d1f4f93e0f 100644 > --- a/tools/kwbimage.c > +++ b/tools/kwbimage.c > @@ -280,14 +280,6 @@ static uint8_t image_checksum8(void *start, uint32_t len) > return csum; > } > > -size_t kwbimage_header_size(unsigned char *ptr) > -{ > - if (kwbimage_version((void *)ptr) == 0) > - return sizeof(struct main_hdr_v0); > - else > - return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr); > -} > - > /* > * Verify checksum over a complete header that includes the checksum field. > * Return 1 when OK, otherwise 0. > @@ -298,7 +290,7 @@ static int main_hdr_checksum_ok(void *hdr) > struct main_hdr_v0 *main_hdr = (struct main_hdr_v0 *)hdr; > uint8_t checksum; > > - checksum = image_checksum8(hdr, kwbimage_header_size(hdr)); > + checksum = image_checksum8(hdr, kwbheader_size_for_csum(hdr)); > /* Calculated checksum includes the header checksum field. Compensate > * for that. > */ > @@ -1649,8 +1641,8 @@ static int kwbimage_check_image_types(uint8_t type) > static int kwbimage_verify_header(unsigned char *ptr, int image_size, > struct image_tool_params *params) > { > - uint8_t checksum; > - size_t header_size = kwbimage_header_size(ptr); > + size_t header_size = kwbheader_size(ptr); > + uint8_t csum; > > if (header_size > image_size) > return -FDT_ERR_BADSTRUCTURE; > @@ -1663,17 +1655,10 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, > struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr; > > if (mhdr->ext & 0x1) { > - struct ext_hdr_v0 *ext_hdr; > - > - if (header_size + sizeof(*ext_hdr) > image_size) > - return -FDT_ERR_BADSTRUCTURE; > + struct ext_hdr_v0 *ext_hdr = (void *)(mhdr + 1); > > - ext_hdr = (struct ext_hdr_v0 *) > - (ptr + sizeof(struct main_hdr_v0)); > - checksum = image_checksum8(ext_hdr, > - sizeof(struct ext_hdr_v0) > - - sizeof(uint8_t)); > - if (checksum != ext_hdr->checksum) > + csum = image_checksum8(ext_hdr, sizeof(*ext_hdr) - 1); > + if (csum != ext_hdr->checksum) > return -FDT_ERR_BADSTRUCTURE; > } > } else if (kwbimage_version(ptr) == 1) { > @@ -1832,7 +1817,7 @@ static int kwbimage_generate(struct image_tool_params *params, > static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params) > { > struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; > - size_t header_size = kwbimage_header_size(ptr); > + size_t header_size = kwbheader_size(ptr); > struct opt_hdr_v1 *ohdr; > int idx = params->pflag; > int cur_idx = 0; > diff --git a/tools/kwbimage.h b/tools/kwbimage.h > index 738034beb1..56a748d4cf 100644 > --- a/tools/kwbimage.h > +++ b/tools/kwbimage.h > @@ -69,11 +69,6 @@ struct ext_hdr_v0 { > uint8_t checksum; > } __packed; > > -struct kwb_header { > - struct main_hdr_v0 kwb_hdr; > - struct ext_hdr_v0 kwb_exthdr; > -} __packed; > - > /* Structure of the main header, version 1 (Armada 370/38x/XP) */ > struct main_hdr_v1 { > uint8_t blockid; /* 0x0 */ > @@ -195,13 +190,6 @@ struct register_set_hdr_v1 { > #define OPT_HDR_V1_BINARY_TYPE 0x2 > #define OPT_HDR_V1_REGISTER_TYPE 0x3 > > -#define KWBHEADER_V0_SIZE(hdr) \ > - (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \ > - sizeof(struct main_hdr_v0)) > - > -#define KWBHEADER_V1_SIZE(hdr) \ > - (((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb)) > - > enum kwbimage_cmd { > CMD_INVALID, > CMD_BOOT_FROM, > @@ -235,6 +223,29 @@ static inline unsigned int kwbimage_version(const void *header) > return ptr[8]; > } > > +static inline size_t kwbheader_size(const void *header) > +{ > + if (kwbimage_version(header) == 0) { > + const struct main_hdr_v0 *hdr = header; > + > + return sizeof(*hdr) + > + (hdr->ext & 0x1) ? sizeof(struct ext_hdr_v0) : 0; > + } else { > + const struct main_hdr_v1 *hdr = header; > + > + return (hdr->headersz_msb << 16) | > + le16_to_cpu(hdr->headersz_lsb); > + } > +} > + > +static inline size_t kwbheader_size_for_csum(const void *header) > +{ > + if (kwbimage_version(header) == 0) > + return sizeof(struct main_hdr_v0); > + else > + return kwbheader_size(header); > +} > + > static inline uint32_t opt_hdr_v1_size(const struct opt_hdr_v1 *ohdr) > { > return (ohdr->headersz_msb << 16) | le16_to_cpu(ohdr->headersz_lsb); > diff --git a/tools/kwboot.c b/tools/kwboot.c > index e47bf94e89..e60f7c5b7a 100644 > --- a/tools/kwboot.c > +++ b/tools/kwboot.c > @@ -583,10 +583,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size) > int rc, pnum; > size_t hdrsz; > > - if (kwbimage_version(img) == 0) > - hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img); > - else > - hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img); > + hdrsz = kwbheader_size(img); > > kwboot_printv("Waiting 2s and flushing tty\n"); > sleep(2); /* flush isn't effective without it */ > @@ -746,9 +743,13 @@ out: > } > > static uint8_t > -kwboot_img_csum8(void *_data, size_t size) > +kwboot_hdr_csum8(const void *hdr) > { > - uint8_t *data = _data, csum; > + const uint8_t *data = hdr; > + uint8_t csum; > + size_t size; > + > + size = kwbheader_size_for_csum(hdr); > > for (csum = 0; size-- > 0; data++) > csum += *data; > @@ -794,17 +795,14 @@ kwboot_img_patch_hdr(void *img, size_t size) > goto out; > } > > - if (image_ver == 0) > - hdrsz = sizeof(*hdr); > - else > - hdrsz = KWBHEADER_V1_SIZE(hdr); > + hdrsz = kwbheader_size(hdr); > > if (size < hdrsz) { > errno = EINVAL; > goto out; > } > > - csum = kwboot_img_csum8(hdr, hdrsz) - hdr->checksum; > + csum = kwboot_hdr_csum8(hdr) - hdr->checksum; > if (csum != hdr->checksum) { > errno = EINVAL; > goto out; > @@ -860,7 +858,7 @@ kwboot_img_patch_hdr(void *img, size_t size) > hdr->blockid = IBR_HDR_UART_ID; > } > > - hdr->checksum = kwboot_img_csum8(hdr, hdrsz) - csum; > + hdr->checksum = kwboot_hdr_csum8(hdr) - csum; > > rc = 0; > out: > Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de