From patchwork Fri Jul 14 11:24:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthias Schiffer X-Patchwork-Id: 1807740 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=85.214.62.61; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=tq-group.com header.i=@tq-group.com header.a=rsa-sha256 header.s=key1 header.b=o/A5gs+K; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4R2TgB1PTSz20c1 for ; Fri, 14 Jul 2023 21:25:42 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3D68486CD5; Fri, 14 Jul 2023 13:25:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ew.tq-group.com 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=tq-group.com header.i=@tq-group.com header.b="o/A5gs+K"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DC49F86CC9; Fri, 14 Jul 2023 13:25:19 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.2 Received: from mx1.tq-group.com (mx1.tq-group.com [93.104.207.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id CA06886CBF for ; Fri, 14 Jul 2023 13:25:15 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ew.tq-group.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=matthias.schiffer@ew.tq-group.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1689333915; x=1720869915; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=1Y1fg0xPKMBZ+VGz7v6Lt7mDtv4+sLJ2R+l9r9hOTy8=; b=o/A5gs+KPFH1+tJxn07F/6xvGs2XNcb/T6twp6HWCN7T22t45zGwEeHO 2rvlLrnaL1JcVEOUhyBhg+jY7eelLjU1j+b9xb6fiq8WNqEm60V+x7S1w nZb5KFPhozLeS84sIh++Un4nP1jayGdow6vp1eyKtXMrkyifvnVGQ0fkM kT/aHGnPFOJQk3Pa6wCdTNIohfodbOWo909ypA+tRyLs24S1znGKDgriQ LYJwONaZ2RxEr4Nk4lMO7UEmROGMiB9XaHklJgkCpjQYF4H2AJHfyIc6T GlqNvEGwtmfzB/2i7VDeNhi3taXwj9X84v5JRPdNVqnJfspEOjyYV6YLC A==; X-IronPort-AV: E=Sophos;i="6.01,205,1684792800"; d="scan'208";a="31936101" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 14 Jul 2023 13:25:15 +0200 Received: from localhost.localdomain (SCHIFFERM-M2.tq-net.de [10.121.49.20]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 233B1280078; Fri, 14 Jul 2023 13:25:15 +0200 (CEST) From: Matthias Schiffer To: u-boot@lists.denx.de Cc: Simon Glass , Sean Anderson , Masahisa Kojima , u-boot-amlogic@groups.io, u-boot@ew.tq-group.com, Matthias Schiffer Subject: [PATCH 1/2] Revert "lib: string: Fix strlcpy return value", fix callers Date: Fri, 14 Jul 2023 13:24:50 +0200 Message-Id: <20230714112451.144294-1-matthias.schiffer@ew.tq-group.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.8 at phobos.denx.de X-Virus-Status: Clean Both the Linux kernel and libbsd agree that strlcpy() should always return strlen(src) and not include the NUL termination. The incorrect U-Boot implementation makes it impossible to check the return value for truncation, and breaks code written with the usual implementation in mind (for example, fdtdec_add_reserved_memory() was subtly broken). I reviewed all callers of strlcpy() and strlcat() and fixed them according to my understanding of the intended function. This reverts commit d3358ecc54be0bc3b4dd11f7a63eab0a2842f772 and adds related fixes. Fixes: d3358ecc54be ("lib: string: Fix strlcpy return value") Signed-off-by: Matthias Schiffer Reviewed-by: Simon Glass Reviewed-by: Sean Anderson --- board/amlogic/vim3/vim3.c | 6 +++--- drivers/fastboot/fb_getvar.c | 2 +- lib/string.c | 14 +++++++------- test/lib/strlcat.c | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/board/amlogic/vim3/vim3.c b/board/amlogic/vim3/vim3.c index fcd60ab1e05..8bdfb302f72 100644 --- a/board/amlogic/vim3/vim3.c +++ b/board/amlogic/vim3/vim3.c @@ -104,8 +104,8 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Update PHY names (mandatory to disable USB3.0) */ - len = strlcpy(data, "usb2-phy0", 32); - len += strlcpy(&data[len], "usb2-phy1", 32 - len); + len = strlcpy(data, "usb2-phy0", 32) + 1; + len += strlcpy(&data[len], "usb2-phy1", 32 - len) + 1; ret = fdt_setprop(blob, node, "phy-names", data, len); if (ret < 0) { printf("vim3: failed to update usb phy names property (%d)\n", ret); @@ -132,7 +132,7 @@ int meson_ft_board_setup(void *blob, struct bd_info *bd) } /* Enable PCIe */ - len = strlcpy(data, "okay", 32); + len = strlcpy(data, "okay", 32) + 1; ret = fdt_setprop(blob, node, "status", data, len); if (ret < 0) { printf("vim3: failed to enable pcie node (%d)\n", ret); diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index dd3475e0a8b..d9f0f07b2bc 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -183,7 +183,7 @@ static void __maybe_unused getvar_has_slot(char *part_name, char *response) /* part_name_wslot = part_name + "_a" */ len = strlcpy(part_name_wslot, part_name, PART_NAME_LEN - 3); - if (len > PART_NAME_LEN - 3) + if (len >= PART_NAME_LEN - 3) goto fail; strcat(part_name_wslot, "_a"); diff --git a/lib/string.c b/lib/string.c index ecea755f405..f2c61471288 100644 --- a/lib/string.c +++ b/lib/string.c @@ -116,20 +116,18 @@ char * strncpy(char * dest,const char *src,size_t count) * of course, the buffer size is zero). It does not pad * out the result like strncpy() does. * - * Return: the number of bytes copied + * Return: strlen(src) */ size_t strlcpy(char *dest, const char *src, size_t size) { - if (size) { - size_t srclen = strlen(src); - size_t len = (srclen >= size) ? size - 1 : srclen; + size_t ret = strlen(src); + if (size) { + size_t len = (ret >= size) ? size - 1 : ret; memcpy(dest, src, len); dest[len] = '\0'; - return len + 1; } - - return 0; + return ret; } #endif @@ -191,6 +189,8 @@ char * strncat(char *dest, const char *src, size_t count) * Compatible with *BSD: the result is always a valid NUL-terminated string that * fits in the buffer (unless, of course, the buffer size is zero). It does not * write past @size like strncat() does. + * + * Return: min(strlen(dest), size) + strlen(src) */ size_t strlcat(char *dest, const char *src, size_t size) { diff --git a/test/lib/strlcat.c b/test/lib/strlcat.c index a0ec037388b..d8453fe78e2 100644 --- a/test/lib/strlcat.c +++ b/test/lib/strlcat.c @@ -43,11 +43,11 @@ static int do_test_strlcat(struct unit_test_state *uts, int line, size_t align1, s2[i] = 32 + 23 * i % (127 - 32); s2[len2 - 1] = '\0'; - expected = len2 < n ? min(len1 + len2 - 1, n) : n; + expected = min(strlen(s2), n) + strlen(s1); actual = strlcat(s2, s1, n); if (expected != actual) { ut_failf(uts, __FILE__, line, __func__, - "strlcat(s2, s1, 2) == len2 < n ? min(len1 + len2, n) : n", + "strlcat(s2, s1, n) == min(len2, n) + len1", "Expected %#zx (%zd), got %#zx (%zd)", expected, expected, actual, actual); return CMD_RET_FAILURE; From patchwork Fri Jul 14 11:24:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthias Schiffer X-Patchwork-Id: 1807739 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.denx.de (client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; helo=phobos.denx.de; envelope-from=u-boot-bounces@lists.denx.de; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=tq-group.com header.i=@tq-group.com header.a=rsa-sha256 header.s=key1 header.b=pF4vyNUF; dkim-atps=neutral Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4R2Tg020CJz20c1 for ; Fri, 14 Jul 2023 21:25:30 +1000 (AEST) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1DD8286CC3; Fri, 14 Jul 2023 13:25:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ew.tq-group.com 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=tq-group.com header.i=@tq-group.com header.b="pF4vyNUF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 39DD886CCE; Fri, 14 Jul 2023 13:25:19 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.2 Received: from mx1.tq-group.com (mx1.tq-group.com [93.104.207.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 321D886CC9 for ; Fri, 14 Jul 2023 13:25:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ew.tq-group.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=matthias.schiffer@ew.tq-group.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1689333916; x=1720869916; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=b6LfGCrYWYDTpOabgNEo6uJOS3nJVmz8FaPR4c/f6uE=; b=pF4vyNUFTvR30uTf7zdNMNjD/T4U02OyLubSwqpEbnGymklKAMrdSAYw AKs7BZqumcpmj6Zoap99Cv8gDzhwIRel7HLkXYpVMO9QYlV9qQdKVq+Ky KHtNXgbeeIiQGbAdaFA37bXWHkNmMqzMsyY9j4Kkr6EVa1ahDcgtZ3brh BbMQ+jhhhcWWZBPTd1Kra93ctp2q7quf2e4PaA41Ga6JO/UgK6gHio5lv ITNzgkaLUpdgdOm9o9171uRm99waDPZxhK1aIY9030icn0RLcIt5QfvUy yG5KpqNzOC7HxCIVQgq2649GY92vM9my1LZ/svVF6/1O7QLHguwnc7vx8 Q==; X-IronPort-AV: E=Sophos;i="6.01,205,1684792800"; d="scan'208";a="31936102" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 14 Jul 2023 13:25:15 +0200 Received: from localhost.localdomain (SCHIFFERM-M2.tq-net.de [10.121.49.20]) by vtuxmail01.tq-net.de (Postfix) with ESMTPA id 46204280084; Fri, 14 Jul 2023 13:25:15 +0200 (CEST) From: Matthias Schiffer To: u-boot@lists.denx.de Cc: Simon Glass , Sean Anderson , Masahisa Kojima , u-boot-amlogic@groups.io, u-boot@ew.tq-group.com, Matthias Schiffer Subject: [PATCH 2/2] lib/charset: fix u16_strlcat() return value Date: Fri, 14 Jul 2023 13:24:51 +0200 Message-Id: <20230714112451.144294-2-matthias.schiffer@ew.tq-group.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230714112451.144294-1-matthias.schiffer@ew.tq-group.com> References: <20230714112451.144294-1-matthias.schiffer@ew.tq-group.com> MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.8 at phobos.denx.de X-Virus-Status: Clean strlcat returns min(strlen(dest), count)+strlen(src). Make u16_strlcat's behaviour the same for consistency. Fixes: eca08ce94ceb ("lib/charset: add u16_strlcat() function") Signed-off-by: Matthias Schiffer --- lib/charset.c | 8 ++++---- test/unicode_ut.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/charset.c b/lib/charset.c index b1842755eb1..5e4c4f948a4 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -444,14 +444,14 @@ u16 *u16_strdup(const void *src) size_t u16_strlcat(u16 *dest, const u16 *src, size_t count) { - size_t destlen = u16_strlen(dest); + size_t destlen = u16_strnlen(dest, count); size_t srclen = u16_strlen(src); - size_t ret = destlen + srclen + 1; + size_t ret = destlen + srclen; if (destlen >= count) return ret; - if (ret > count) - srclen -= ret - count; + if (ret >= count) + srclen -= (ret - count + 1); memcpy(&dest[destlen], src, 2 * srclen); dest[destlen + srclen] = 0x0000; diff --git a/test/unicode_ut.c b/test/unicode_ut.c index b27d7116b9e..a9356e2b60d 100644 --- a/test/unicode_ut.c +++ b/test/unicode_ut.c @@ -808,12 +808,12 @@ static int unicode_test_u16_strlcat(struct unit_test_state *uts) /* dest and src are empty string */ memset(buf, 0, sizeof(buf)); ret = u16_strlcat(buf, &null_src, sizeof(buf)); - ut_asserteq(1, ret); + ut_asserteq(0, ret); /* dest is empty string */ memset(buf, 0, sizeof(buf)); ret = u16_strlcat(buf, src, sizeof(buf)); - ut_asserteq(5, ret); + ut_asserteq(4, ret); ut_assert(!unicode_test_u16_strcmp(buf, src, 40)); /* src is empty string */ @@ -821,14 +821,14 @@ static int unicode_test_u16_strlcat(struct unit_test_state *uts) buf[39] = 0; memcpy(buf, dest, sizeof(dest)); ret = u16_strlcat(buf, &null_src, sizeof(buf)); - ut_asserteq(6, ret); + ut_asserteq(5, ret); ut_assert(!unicode_test_u16_strcmp(buf, dest, 40)); for (i = 0; i <= 40; i++) { memset(buf, 0xCD, (sizeof(buf) - sizeof(u16))); buf[39] = 0; memcpy(buf, dest, sizeof(dest)); - expected = 10; + expected = min(5, i) + 4; ret = u16_strlcat(buf, src, i); ut_asserteq(expected, ret); if (i <= 6) {