On Thu, Jun 1, 2023 at 11:46 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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 == 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))); I personally prefer this approach, and I agree that it makes the code look nicer. Anna > if (unlikely(!p)) > return -EBADMSG; > if (array == NULL) >
