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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7B73C001E0 for ; Tue, 1 Aug 2023 07:49:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231872AbjHAHtE (ORCPT ); Tue, 1 Aug 2023 03:49:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53626 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231426AbjHAHtD (ORCPT ); Tue, 1 Aug 2023 03:49:03 -0400 Received: from todd.t-8ch.de (todd.t-8ch.de [159.69.126.157]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 38509E48; Tue, 1 Aug 2023 00:49:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=weissschuh.net; s=mail; t=1690876140; bh=qsbOXFqF2xX1Cj3m1X+7zylT1KmXCgIpush39Cl5JtQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bWUrDjWULaOlHXTqUu5UmVfeqHLaTpy2k1zUYBvlYy/zWGzYpIIMO0odL2xdqMWub 89KhGnUxOqLLpURKST/RRSaw1vdZ1EdRxudJofu/e9vi61Fh4nfRHnNV15aI6Hh4oV BRIeXzkTP9f9pOIBvRo8C0a7UzmQscMANtzRGTTc= Date: Tue, 1 Aug 2023 09:48:59 +0200 From: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= To: Willy Tarreau Cc: Shuah Khan , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Yuan Tan , Zhangjin Wu Subject: Re: [PATCH v2 09/10] selftests/nolibc: test return value of read() in test_vfprintf Message-ID: References: <20230801-nolibc-warnings-v2-0-1ba5ca57bd9b@weissschuh.net> <20230801-nolibc-warnings-v2-9-1ba5ca57bd9b@weissschuh.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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.