Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Jun 2023 18:34:53 +0300
From:   Dan Carpenter <dan.carpenter@linaro.org>
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Dmitry Antipov <dmantipov@yandex.ru>,
        Jeff Layton <jlayton@kernel.org>, Tom Rix <trix@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] sunrpc: fix clang-17 warning
Message-ID: <8ed6eb2b-fdfa-4fde-81f3-92e6b34bc509@kadam.mountain>
References: <20230601143332.255312-1-dmantipov@yandex.ru>
 <2D3D2D8E-4E7A-4B50-A1FF-486D7F6C26D4@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2D3D2D8E-4E7A-4B50-A1FF-486D7F6C26D4@oracle.com>
Precedence: bulk

On Thu, Jun 01, 2023 at 02:38:58PM +0000, Chuck Lever III wrote:
> > - if (len > SIZE_MAX / sizeof(*p))
> > + if (unlikely(SIZE_MAX == U32_MAX ? (len > U32_MAX / sizeof(*p)) : 0))
>

This is a bug in Clang.

Generally the rule, is that if there is a bug in the static checker then
you should fix the checker instead of the code.  Smatch used to have
this same bug but I fixed it.  So it's not something which is
unfixable.  This doesn't cause a problem for normal Clang builds, only
for W=1, right?

But, here is a nicer way to fix it.  You can send this, or I can send
it tomorrow with your Reported-by?

regards,
dan carpenter

Fix the following warning observed when building 64-bit (actually arm64)
kernel with clang-17 (make LLVM=1 W=1):

include/linux/sunrpc/xdr.h:779:10: warning: result of comparison of constant
4611686018427387903 with expression of type '__u32' (aka 'unsigned int') is
always false [-Wtautological-constant-out-of-range-compare]
 779 |         if (len > SIZE_MAX / sizeof(*p))

That is, an overflow check makes sense for 32-bit kernel only.  Silence
the Clang warning and make the code nicer by using the size_mul()
function to prevent integer overflows.

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index f89ec4b5ea16..dbf7620a2853 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -775,9 +775,7 @@ xdr_stream_decode_uint32_array(struct xdr_stream *xdr,
 
 	if (unlikely(xdr_stream_decode_u32(xdr, &len) < 0))
 		return -EBADMSG;
-	if (len > SIZE_MAX / sizeof(*p))
-		return -EBADMSG;
-	p = xdr_inline_decode(xdr, len * sizeof(*p));
+	p = xdr_inline_decode(xdr, size_mul(len, sizeof(*p)));
 	if (unlikely(!p))
 		return -EBADMSG;
 	if (array == NULL)