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;
MIME-Version: 1.0
References: <20230601143332.255312-1-dmantipov@yandex.ru> <2D3D2D8E-4E7A-4B50-A1FF-486D7F6C26D4@oracle.com>
 <8ed6eb2b-fdfa-4fde-81f3-92e6b34bc509@kadam.mountain>
In-Reply-To: <8ed6eb2b-fdfa-4fde-81f3-92e6b34bc509@kadam.mountain>
From:   Anna Schumaker <schumaker.anna@gmail.com>
Date:   Thu, 1 Jun 2023 11:56:10 -0400
Message-ID: <CAFX2JfmGvpa2ucp=ZxWm_RszTXGr+50mbkatbscXu=BbVpAwgg@mail.gmail.com>
Subject: Re: [PATCH] sunrpc: fix clang-17 warning
To:     Dan Carpenter <dan.carpenter@linaro.org>
Cc:     Chuck Lever III <chuck.lever@oracle.com>,
        Dmitry Antipov <dmantipov@yandex.ru>,
        Jeff Layton <jlayton@kernel.org>, Tom Rix <trix@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jun 1, 2023 at 11:46=E2=80=AFAM Dan Carpenter <dan.carpenter@linaro=
.org> wrote:
>
> On Thu, Jun 01, 2023 at 02:38:58PM +0000, Chuck Lever III wrote:
> > > - if (len > SIZE_MAX / sizeof(*p))
> > > + if (unlikely(SIZE_MAX =3D=3D 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=3D1, 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=3D1 W=3D1):
>
> include/linux/sunrpc/xdr.h:779:10: warning: result of comparison of const=
ant
> 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 =3D xdr_inline_decode(xdr, len * sizeof(*p));
> +       p =3D xdr_inline_decode(xdr, size_mul(len, sizeof(*p)));

I personally prefer this approach, and I agree that it makes the code
look nicer.

Anna


>         if (unlikely(!p))
>                 return -EBADMSG;
>         if (array =3D=3D NULL)
>