Message ID | 20230801-nolibc-warnings-v2-9-1ba5ca57bd9b@weissschuh.net |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:918b:0:b0:3e4:2afc:c1 with SMTP id s11csp2494330vqg; Tue, 1 Aug 2023 00:13:00 -0700 (PDT) X-Google-Smtp-Source: APBJJlGaMQlMpE6iefYezMorlEeWBk2nkaqT2WGftpr7niZ5Utw+ZPA1YS0TBSzXlrxV2LPHSCj0 X-Received: by 2002:a05:6402:31fa:b0:51e:2305:931 with SMTP id dy26-20020a05640231fa00b0051e23050931mr1688509edb.22.1690873980204; Tue, 01 Aug 2023 00:13:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690873980; cv=none; d=google.com; s=arc-20160816; b=vjZ8Tb3lNMNxDkUHd8ElFgE6bQWYxQeUnkRVKRj16SEir04L2lOS5/B+KH0SOEuCxq ea9Ie6r8KY/tuouFpax8DCwlb1Y1b4ZGIUCrK4CtR9vn7MMJMkMn8fBJAondn2x5T7Yb YD2oDAXyWvjULksONXJ9h8gPU+bKg+qlAl/mxEAhrcWdnyqvg0xjE7IxvRmB50ICPXgL abxk9LWhxL/r/CM4LkhTZcICa+g0la3ik5ZY2/vQhmMLqu3qA7AMxG8b8lsAGeAznVBa OcQUKePZuO4cl4jqVH3KRnd3gjM810I+PPQ9mv/NTaETOmfwD1hFF0mZ5t44oWFQk6rt I28Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=dbzlKwQO6EhBfuZmbJRW2wAuMYhZkgGl3ACnDDz5mFA=; fh=UZfhiU2pgwvZdBOe9fFEhnfTQX0/6KsvXKQubWsfhik=; b=hk9Rn/4zXZQKafaHlg720AkOwrApXYWvN1UubNTr+k9yQCzICPjlTjxDDMnUJTU++B pc2z0psFP7hPILEQVIqw48mCyBosLeQ77XGypAdQsZJkcwDovpUi7HUOKkyMlRiISd+u fs+CKap6xMwmSOnAr7HygEv0bdA7GGTtC4+IlAHfHs4p+p9dkStCfZwewwNMXJusqgXI kj6PUdKGsHI4ISG2h2P72t7C70hyKS+Ma7UUE5mrk9l1M4k7RmZ3FmeSD6+9GKJ3zfd7 TzshUxb+qJT8SFLYGcIgU0Bjc+or5cPI3/YExSSujhhLst4p3fd7SzRbyYg9d2hRTEAf Pw+Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@weissschuh.net header.s=mail header.b=AVLr7ez6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l10-20020aa7c30a000000b00522aaf71df1si5600107edq.687.2023.08.01.00.12.37; Tue, 01 Aug 2023 00:13:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@weissschuh.net header.s=mail header.b=AVLr7ez6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230383AbjHAFau (ORCPT <rfc822;maxi.paulin@gmail.com> + 99 others); Tue, 1 Aug 2023 01:30:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45266 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230188AbjHAFaZ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 1 Aug 2023 01:30:25 -0400 Received: from todd.t-8ch.de (todd.t-8ch.de [159.69.126.157]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F0C31FC3; Mon, 31 Jul 2023 22:30:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=weissschuh.net; s=mail; t=1690867813; bh=+UidgUyTA/KFUXNdvZyyZmaUyZXJ9vuTjGYxg60wA00=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=AVLr7ez6Xso9rIiCq4XROnDn87bAXg6Gu+TqqS5fJxVWzM+cEMh6v8CKhONkYrbVa kUH+qIh/OljtRxtQwTBEKzvhDh381ngp1ZSsnptM/xin7BOHEiFmb2Q3pCi3CP+fS5 IVFICVId0E4ByCfMXC5uw5KQrwlA9d2ce4IUT18g= From: =?utf-8?q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net> Date: Tue, 01 Aug 2023 07:30:16 +0200 Subject: [PATCH v2 09/10] selftests/nolibc: test return value of read() in test_vfprintf MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-Id: <20230801-nolibc-warnings-v2-9-1ba5ca57bd9b@weissschuh.net> References: <20230801-nolibc-warnings-v2-0-1ba5ca57bd9b@weissschuh.net> In-Reply-To: <20230801-nolibc-warnings-v2-0-1ba5ca57bd9b@weissschuh.net> To: Willy Tarreau <w@1wt.eu>, Shuah Khan <shuah@kernel.org> Cc: linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Yuan Tan <tanyuan@tinylab.org>, Zhangjin Wu <falcon@tinylab.org>, =?utf-8?q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net> X-Mailer: b4 0.12.3 X-Developer-Signature: v=1; a=ed25519-sha256; t=1690867811; l=826; i=linux@weissschuh.net; s=20221212; h=from:subject:message-id; bh=+UidgUyTA/KFUXNdvZyyZmaUyZXJ9vuTjGYxg60wA00=; b=1TjhwvvgPvP269jHQXdAQsLEjEYjifMWLDyOHpOqaW6hu+NWX+/3UwMD5u0Shu4CJ5pFD1VCA t/aAkmg/LRNBR4+1k4EukM6GFycmMc4SFbETt0POLETAe6YgDzd9VqM X-Developer-Key: i=linux@weissschuh.net; a=ed25519; pk=KcycQgFPX2wGR5azS7RhpBqedglOZVgRPfdFSPB1LNw= X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_PASS,T_SCC_BODY_TEXT_LINE, T_SPF_HELO_TEMPERROR,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773009874832017814 X-GMAIL-MSGID: 1773009874832017814 |
Series |
tools/nolibc: enable compiler warnings
|
|
Commit Message
Thomas Weißschuh
Aug. 1, 2023, 5:30 a.m. UTC
If read() fails and returns -1 buf would be accessed out of bounds.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
tools/testing/selftests/nolibc/nolibc-test.c | 6 ++++++
1 file changed, 6 insertions(+)
Comments
On Tue, Aug 01, 2023 at 07:30:16AM +0200, Thomas Weißschuh wrote: > If read() fails and returns -1 buf would be accessed out of bounds. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > tools/testing/selftests/nolibc/nolibc-test.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c > index 82714051c72f..a334f8450a34 100644 > --- a/tools/testing/selftests/nolibc/nolibc-test.c > +++ b/tools/testing/selftests/nolibc/nolibc-test.c > @@ -1031,6 +1031,12 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm > lseek(fd, 0, SEEK_SET); > > r = read(fd, buf, sizeof(buf) - 1); > + if (r == -1) { > + llen += printf(" read() = %s", errorname(errno)); > + result(llen, FAIL); > + return 1; > + } > + > buf[r] = '\0'; In fact given the nature of this file (test if we properly implemented our syscalls), I think that a more conservative approach is deserved because if we messed up on read() we can have anything on return and we don't want to trust that. As such I would suggest that we declare r as ssize_t and verify that it's neither negative nor larger than sizeof(buf)-1, which becomes: if ((size_t)r >= sizeof(buf)) { ... fail ... } You'll also have to turn w to ssize_t then due to the test later BTW. Willy
On 2023-08-01 08:59:17+0200, Willy Tarreau wrote: > On Tue, Aug 01, 2023 at 07:30:16AM +0200, Thomas Weißschuh wrote: > > If read() fails and returns -1 buf would be accessed out of bounds. > > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > --- > > tools/testing/selftests/nolibc/nolibc-test.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c > > index 82714051c72f..a334f8450a34 100644 > > --- a/tools/testing/selftests/nolibc/nolibc-test.c > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c > > @@ -1031,6 +1031,12 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm > > lseek(fd, 0, SEEK_SET); > > > > r = read(fd, buf, sizeof(buf) - 1); > > + if (r == -1) { > > + llen += printf(" read() = %s", errorname(errno)); > > + result(llen, FAIL); > > + return 1; > > + } > > + > > buf[r] = '\0'; > > In fact given the nature of this file (test if we properly implemented > our syscalls), I think that a more conservative approach is deserved > because if we messed up on read() we can have anything on return and we > don't want to trust that. As such I would suggest that we declare r as > ssize_t and verify that it's neither negative nor larger than > sizeof(buf)-1, which becomes: > > if ((size_t)r >= sizeof(buf)) { > ... fail ... > } As r == w is validated just below anyways we could move the assignment buf[r] = '\0' after that check and then we don't need a new block. > You'll also have to turn w to ssize_t then due to the test later BTW. Will do in any case.
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 82714051c72f..a334f8450a34 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -1031,6 +1031,12 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm lseek(fd, 0, SEEK_SET); r = read(fd, buf, sizeof(buf) - 1); + if (r == -1) { + llen += printf(" read() = %s", errorname(errno)); + result(llen, FAIL); + return 1; + } + buf[r] = '\0'; fclose(memfile);