[PATCH 0/9] address remaining -Wtautological-constant-out-of-range-compare

[PATCH 0/9] address remaining -Wtautological-constant-out-of-range-compare

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The warning option was introduced a few years ago but left disabled
by default. All of the actual bugs that this has found have been
fixed in the meantime, and this series should address the remaining
false-positives, as tested on arm/arm64/x86 randconfigs as well as
allmodconfig builds for all architectures supported by clang.

Please apply the patches individually to subsystem maintainer trees.

      Arnd

Arnd Bergmann (9):
  dm integrity: fix out-of-range warning
  libceph: avoid clang out-of-range warning
  rbd: avoid out-of-range warning
  kcov: avoid clang out-of-range warning
  ipv4: tcp_output: avoid warning about NET_ADD_STATS
  nilfs2: fix out-of-range warning
  infiniband: uverbs: avoid out-of-range warnings
  mlx5: stop warning for 64KB pages
  kbuild: enable tautological-constant-out-of-range-compare

 drivers/block/rbd.c                                    | 2 +-
 drivers/infiniband/core/uverbs_ioctl.c                 | 4 ++--
 drivers/md/dm-integrity.c                              | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++--
 fs/ceph/snap.c                                         | 2 +-
 fs/nilfs2/ioctl.c                                      | 2 +-
 kernel/kcov.c                                          | 3 ++-
 net/ceph/osdmap.c                                      | 4 ++--
 net/ipv4/tcp_output.c                                  | 2 +-
 scripts/Makefile.extrawarn                             | 1 -
 10 files changed, 15 insertions(+), 13 deletions(-)

-- 
2.39.2

Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: Dongsheng Yang <dongsheng.yang@easystack.cn>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@kernel.org>
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: dm-devel@lists.linux.dev
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Xiubo Li <xiubli@redhat.com>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: David Ahern <dsahern@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nicolas Schier <nicolas@fjasle.eu>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: ceph-devel@vger.kernel.org
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-nilfs@vger.kernel.org
Cc: kasan-dev@googlegroups.com
Cc: linux-kbuild@vger.kernel.org
Cc: llvm@lists.linux.dev

[PATCH 1/9] dm integrity: fix out-of-range warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Depending on the value of CONFIG_HZ, clang compares about a pointless
comparison:

drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
                        if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {

As the check remains useful for other configurations, shut up the
warning by adding a second type cast to uint64_t.

Fixes: 468dfca38b1a ("dm integrity: add a bitmap mode")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/md/dm-integrity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 37b9f8f1ae1a..7f3dc8ee6ab8 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		} else if (sscanf(opt_string, "sectors_per_bit:%llu%c", &llval, &dummy) == 1) {
 			log2_sectors_per_bitmap_bit = !llval ? 0 : __ilog2_u64(llval);
 		} else if (sscanf(opt_string, "bitmap_flush_interval:%u%c", &val, &dummy) == 1) {
-			if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {
+			if ((uint64_t)val >= (uint64_t)UINT_MAX * 1000 / HZ) {
 				r = -EINVAL;
 				ti->error = "Invalid bitmap_flush_interval argument";
 				goto bad;
-- 
2.39.2

[PATCH 2/9] libceph: avoid clang out-of-range warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang-14 points out that on 64-bit architectures, a u32
is never larger than constant based on SIZE_MAX:

net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
            ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
            ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The code is correct anyway, so just shut up that warning.

Fixes: 6f428df47dae ("libceph: pg_upmap[_items] infrastructure")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/ceph/snap.c    | 2 +-
 net/ceph/osdmap.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
index c65f2b202b2b..521507ea8260 100644
--- a/fs/ceph/snap.c
+++ b/fs/ceph/snap.c
@@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,
 
 	/* alloc new snap context */
 	err = -ENOMEM;
-	if (num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
+	if ((size_t)num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
 		goto fail;
 	snapc = ceph_create_snap_context(num, GFP_NOFS);
 	if (!snapc)
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index 295098873861..8e7cb2fde6f1 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,
 	ceph_decode_32_safe(p, end, len, e_inval);
 	if (len == 0 && incremental)
 		return NULL;	/* new_pg_temp: [] to remove */
-	if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
+	if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
 		return ERR_PTR(-EINVAL);
 
 	ceph_decode_need(p, end, len * sizeof(u32), e_inval);
@@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,
 	u32 len, i;
 
 	ceph_decode_32_safe(p, end, len, e_inval);
-	if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
+	if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
 		return ERR_PTR(-EINVAL);
 
 	ceph_decode_need(p, end, 2 * len * sizeof(u32), e_inval);
-- 
2.39.2

[PATCH 3/9] rbd: avoid out-of-range warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang-14 points out that the range check is always true on 64-bit
architectures since a u32 is not greater than the allowed size:

drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
            ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is harmless, so just change the type of the temporary to size_t
to shut up that warning.

Fixes: bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/block/rbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 26ff5cd2bf0a..cb25ee513ada 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev,
 	void *p;
 	void *end;
 	u64 seq;
-	u32 snap_count;
+	size_t snap_count;
 	struct ceph_snap_context *snapc;
 	u32 i;
 
-- 
2.39.2

[PATCH 4/9] kcov: avoid clang out-of-range warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The area_size is never larger than the maximum on 64-bit architectutes:

kernel/kcov.c:634:29: error: result of comparison of constant 1152921504606846975 with expression of type '__u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
                if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long))
                    ~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The compiler can correctly optimize the check away and the code appears
correct to me, so just add a cast to avoid the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/kcov.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/kcov.c b/kernel/kcov.c
index f9ac2e9e460f..c3124f6d5536 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -627,7 +627,8 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		mode = kcov_get_mode(remote_arg->trace_mode);
 		if (mode < 0)
 			return mode;
-		if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long))
+		if ((unsigned long)remote_arg->area_size >
+		    LONG_MAX / sizeof(unsigned long))
 			return -EINVAL;
 		kcov->mode = mode;
 		t->kcov = kcov;
-- 
2.39.2

[PATCH 5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Clang warns about a range check in percpu_add_op() being impossible
to hit for an u8 variable:

net/ipv4/tcp_output.c:188:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
                NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/ip.h:291:41: note: expanded from macro 'NET_ADD_STATS'
 #define NET_ADD_STATS(net, field, adnd) SNMP_ADD_STATS((net)->mib.net_statistics, field, adnd)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/snmp.h:143:4: note: expanded from macro 'SNMP_ADD_STATS'
                        this_cpu_add(mib->mibs[field], addend)
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/percpu-defs.h:509:33: note: expanded from macro 'this_cpu_add'
 #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
<scratch space>:187:1: note: expanded from here
this_cpu_add_8
^
arch/x86/include/asm/percpu.h:326:35: note: expanded from macro 'this_cpu_add_8'
 #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
                              ((val) == 1 || (val) == -1)) ?            \
                                             ~~~~~ ^  ~~

Avoid this warning with a cast to a signed 'int'.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/ipv4/tcp_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index e3167ad96567..dbe54fceee08 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -183,7 +183,7 @@ static inline void tcp_event_ack_sent(struct sock *sk, u32 rcv_nxt)
 
 	if (unlikely(tp->compressed_ack)) {
 		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
-			      tp->compressed_ack);
+			      (int)tp->compressed_ack);
 		tp->compressed_ack = 0;
 		if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
 			__sock_put(sk);
-- 
2.39.2

[PATCH 6/9] nilfs2: fix out-of-range warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang-14 points out that v_size is always smaller than a 64KB
page size if that is configured by the CPU architecture:

fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (argv->v_size > PAGE_SIZE)
            ~~~~~~~~~~~~ ^ ~~~~~~~~~

This is ok, so just shut up that warning with a cast.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/nilfs2/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index f1a01c191cf5..8be471ce4f19 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
 	if (argv->v_nmembs == 0)
 		return 0;
 
-	if (argv->v_size > PAGE_SIZE)
+	if ((size_t)argv->v_size > PAGE_SIZE)
 		return -EINVAL;
 
 	/*
-- 
2.39.2

[PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang warns for comparisons that are always true, which is the case
for these two page size checks on architectures with 64KB pages:

drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE'
        int __ret_warn_on = !!(condition);                      \
                               ^~~~~~~~~
drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (hdr.length > PAGE_SIZE ||
            ~~~~~~~~~~ ^ ~~~~~~~~~

Add a cast to u32 in both cases, so it never warns about this.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/core/uverbs_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index f80da6a67e24..e0cc3ddae71b 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -90,7 +90,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,
 		ALIGN(bundle_size + 256, sizeof(*pbundle->internal_buffer));
 
 	/* Do not want order-2 allocations for this. */
-	WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
+	WARN_ON_ONCE((u32)method_elm->bundle_size > PAGE_SIZE);
 }
 
 /**
@@ -636,7 +636,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	if (err)
 		return -EFAULT;
 
-	if (hdr.length > PAGE_SIZE ||
+	if ((u32)hdr.length > PAGE_SIZE ||
 	    hdr.length != struct_size(&hdr, attrs, hdr.num_attrs))
 		return -EINVAL;
 
-- 
2.39.2

[PATCH 8/9] mlx5: stop warning for 64KB pages

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

When building with 64KB pages, clang points out that xsk->chunk_size
can never be PAGE_SIZE:

drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (xsk->chunk_size > PAGE_SIZE ||
            ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~

In older versions of this code, using PAGE_SIZE was the only
possibility, so this would have never worked on 64KB page kernels,
but the patch apparently did not address this case completely.

As Maxim Mikityanskiy suggested, 64KB chunks are really not all that
useful, so just shut up the warning by adding a cast.

Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")
Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/
Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
index 06592b9f0424..9240cfe25d10 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
@@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params,
 			      struct mlx5e_xsk_param *xsk,
 			      struct mlx5_core_dev *mdev)
 {
-	/* AF_XDP doesn't support frames larger than PAGE_SIZE. */
-	if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {
+	/* AF_XDP doesn't support frames larger than PAGE_SIZE,
+	 * and xsk->chunk_size is limited to 65535 bytes.
+	 */
+	if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {
 		mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size,
 			      MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE);
 		return false;
-- 
2.39.2

[PATCH 9/9] kbuild: enable tautological-constant-out-of-range-compare

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

All the previously reported warnings from this option have been addressed,
so the option can now be left default-enabled, rather than disabled without
W=1. There are not too many actual bugs found by this, but it can help
detect a silly mistake earlier, and it's usually trivial to work around
the false-positives.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 scripts/Makefile.extrawarn | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 5a25f133d0e9..24d29e477644 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -120,7 +120,6 @@ KBUILD_CFLAGS += -Wformat-insufficient-args
 endif
 endif
 KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
-KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
 KBUILD_CFLAGS += $(call cc-disable-warning, unaligned-access)
 KBUILD_CFLAGS += -Wno-enum-compare-conditional
 KBUILD_CFLAGS += -Wno-enum-enum-conversion
-- 
2.39.2

Re: [PATCH 5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS

[Eric Dumazet]

On Thu, Mar 28, 2024 at 3:31 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Clang warns about a range check in percpu_add_op() being impossible
> to hit for an u8 variable:
>
> net/ipv4/tcp_output.c:188:3: error: result of comparison of constant -1 with expression of type 'u8' (aka 'unsigned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>                 NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/net/ip.h:291:41: note: expanded from macro 'NET_ADD_STATS'
>  #define NET_ADD_STATS(net, field, adnd) SNMP_ADD_STATS((net)->mib.net_statistics, field, adnd)
>                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/net/snmp.h:143:4: note: expanded from macro 'SNMP_ADD_STATS'
>                         this_cpu_add(mib->mibs[field], addend)
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/percpu-defs.h:509:33: note: expanded from macro 'this_cpu_add'
>  #define this_cpu_add(pcp, val)          __pcpu_size_call(this_cpu_add_, pcp, val)
>                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
> <scratch space>:187:1: note: expanded from here
> this_cpu_add_8
> ^
> arch/x86/include/asm/percpu.h:326:35: note: expanded from macro 'this_cpu_add_8'
>  #define this_cpu_add_8(pcp, val)                percpu_add_op(8, volatile, (pcp), val)
>                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
>                               ((val) == 1 || (val) == -1)) ?            \
>                                              ~~~~~ ^  ~~
>

This seems like a bug in the macro or the compiler, because val is not
a constant ?

__builtin_constant_p(val) should return false ???

+#define percpu_add_op(size, qual, var, val)                            \
+do {                                                                   \
+       const int pao_ID__ = (__builtin_constant_p(val) &&              \
+                             ((val) == 1 || (val) == -1)) ?            \
+                               (int)(val) : 0;                         \
+       if (0) {                                                        \
+               typeof(var) pao_tmp__;                                  \
+               pao_tmp__ = (val);                                      \
+               (void)pao_tmp__;                                        \
+       }                                                               \
+       if (pao_ID__ == 1)                                              \
+               percpu_unary_op(size, qual, "inc", var);                \
+       else if (pao_ID__ == -1)                                        \
+               percpu_unary_op(size, qual, "dec", var);                \
+       else                                                            \
+               percpu_to_op(size, qual, "add", var, val);              \
+} while (0)
+



> Avoid this warning with a cast to a signed 'int'.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  net/ipv4/tcp_output.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index e3167ad96567..dbe54fceee08 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -183,7 +183,7 @@ static inline void tcp_event_ack_sent(struct sock *sk, u32 rcv_nxt)
>
>         if (unlikely(tp->compressed_ack)) {
>                 NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
> -                             tp->compressed_ack);
> +                             (int)tp->compressed_ack);
>                 tp->compressed_ack = 0;
>                 if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
>                         __sock_put(sk);
> --
> 2.39.2
>

Re: [PATCH 3/9] rbd: avoid out-of-range warning

[Alex Elder]

On 3/28/24 9:30 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang-14 points out that the range check is always true on 64-bit
> architectures since a u32 is not greater than the allowed size:
> 
> drivers/block/rbd.c:6079:17: error: result of comparison of constant 2305843009213693948 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
w
>              ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This is harmless, so just change the type of the temporary to size_t
> to shut up that warning.

This fixes the warning, but then the now size_t value is passed
to ceph_decode_32_safe(), which implies a different type conversion.
That too is not harmful, but...

Could we just cast the value in the comparison instead?

   if ((size_t)snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))

You could drop the space between sizeof and ( while
you're at it (I always used the space back then).

					-Alex

> 
> Fixes: bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/block/rbd.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 26ff5cd2bf0a..cb25ee513ada 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(struct rbd_device *rbd_dev,
>   	void *p;
>   	void *end;
>   	u64 seq;
> -	u32 snap_count;
> +	size_t snap_count;
>   	struct ceph_snap_context *snapc;
>   	u32 i;
>   

Re: [PATCH 6/9] nilfs2: fix out-of-range warning

[Philipp Stanner]

On Thu, 2024-03-28 at 15:30 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang-14 points out that v_size is always smaller than a 64KB
> page size if that is configured by the CPU architecture:
> 
> fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant
> 65536 with expression of type '__u16' (aka 'unsigned short') is
> always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (argv->v_size > PAGE_SIZE)
>             ~~~~~~~~~~~~ ^ ~~~~~~~~~
> 
> This is ok, so just shut up that warning with a cast.

nit:
It's not a warning, but actually a compile error, right?

(no idea why they make that an error btw. Warning would be perfectly
fine)

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Should / could that be backported to stable kernels in case people
start building those with clang-14?

Regards,
P.

> ---
>  fs/nilfs2/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index f1a01c191cf5..8be471ce4f19 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs
> *nilfs,
>         if (argv->v_nmembs == 0)
>                 return 0;
>  
> -       if (argv->v_size > PAGE_SIZE)
> +       if ((size_t)argv->v_size > PAGE_SIZE)
>                 return -EINVAL;
>  
>         /*

Re: [PATCH 8/9] mlx5: stop warning for 64KB pages

[Maxim Mikityanskiy]

On Thu, 28 Mar 2024 at 15:30:46 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When building with 64KB pages, clang points out that xsk->chunk_size
> can never be PAGE_SIZE:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (xsk->chunk_size > PAGE_SIZE ||
>             ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~
> 
> In older versions of this code, using PAGE_SIZE was the only
> possibility, so this would have never worked on 64KB page kernels,
> but the patch apparently did not address this case completely.
> 
> As Maxim Mikityanskiy suggested, 64KB chunks are really not all that
> useful, so just shut up the warning by adding a cast.
> 
> Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")
> Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/
> Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> index 06592b9f0424..9240cfe25d10 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> @@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params,
>  			      struct mlx5e_xsk_param *xsk,
>  			      struct mlx5_core_dev *mdev)
>  {
> -	/* AF_XDP doesn't support frames larger than PAGE_SIZE. */
> -	if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {
> +	/* AF_XDP doesn't support frames larger than PAGE_SIZE,
> +	 * and xsk->chunk_size is limited to 65535 bytes.
> +	 */
> +	if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {

Acked-by: Maxim Mikityanskiy <maxtram95@gmail.com>

>  		mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size,
>  			      MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE);
>  		return false;
> -- 
> 2.39.2
> 

Re: [PATCH 6/9] nilfs2: fix out-of-range warning

[Arnd Bergmann]

On Thu, Mar 28, 2024, at 16:21, Philipp Stanner wrote:
> On Thu, 2024-03-28 at 15:30 +0100, Arnd Bergmann wrote:
>> 
>> This is ok, so just shut up that warning with a cast.
>
> nit:
> It's not a warning, but actually a compile error, right?

I build with CONFIG_WERROR=y, which turns all warnings
into errors. It's just a warning without that, and is
currently only enabled when building with 'make W=1',
though the point of my series is to have it always enabled.

> (no idea why they make that an error btw. Warning would be perfectly
> fine)
>
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Should / could that be backported to stable kernels in case people
> start building those with clang-14?

It's clearly harmless and could be backported, but it
is not needed either since older kernels will keep the
option as part of W=1, not the default.

      Arnd

Re: [PATCH 5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS

[Arnd Bergmann]

On Thu, Mar 28, 2024, at 15:38, Eric Dumazet wrote:
> On Thu, Mar 28, 2024 at 3:31 PM Arnd Bergmann <arnd@kernel.org> wrote:
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/include/asm/percpu.h:127:31: note: expanded from macro 'percpu_add_op'
>>                               ((val) == 1 || (val) == -1)) ?            \
>>                                              ~~~~~ ^  ~~
>>
>
> This seems like a bug in the macro or the compiler, because val is not
> a constant ?
>
> __builtin_constant_p(val) should return false ???
>
> +#define percpu_add_op(size, qual, var, val)                            \
> +do {                                                                   \
> +       const int pao_ID__ = (__builtin_constant_p(val) &&              \
> +                             ((val) == 1 || (val) == -1)) ?            \
> +                               (int)(val) : 0;                         \

It looks like gcc does the same thing, with the broader and
still disabled -Wtype-limits, see: https://godbolt.org/z/3EPTGx68n

As far as I can tell, it does not matter that the comparison
against -1 is never actually evaluated, since the warning
is already printed before it simplifies the condition.

This is the only such warning I got from percpu, but
I guess we could also add the cast inside of the macro,
such as

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 44958ebaf626..5923d786e67a 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -181,12 +181,14 @@ do {                                                                      \
  */
 #define percpu_add_op(size, qual, var, val)                            \
 do {                                                                   \
-       const int pao_ID__ = (__builtin_constant_p(val) &&              \
-                             ((val) == 1 || (val) == -1)) ?            \
-                               (int)(val) : 0;                         \
+       __auto_type __val = (val);                                      \
+       const int pao_ID__ = (__builtin_constant_p(__val) &&            \
+                             ((__val) == (typeof(__val))1 ||           \
+                              (__val) == (typeof(__val))-1)) ?         \
+                               (int)(__val) : 0;                       \
        if (0) {                                                        \
                typeof(var) pao_tmp__;                                  \
-               pao_tmp__ = (val);                                      \
+               pao_tmp__ = (__val);                                    \
                (void)pao_tmp__;                                        \
        }                                                               \
        if (pao_ID__ == 1)                                              \
@@ -194,7 +196,7 @@ do {                                                                        \
        else if (pao_ID__ == -1)                                        \
                percpu_unary_op(size, qual, "dec", var);                \
        else                                                            \
-               percpu_to_op(size, qual, "add", var, val);              \
+               percpu_to_op(size, qual, "add", var, __val);            \
 } while (0)
 
 #define percpu_from_op(size, qual, op, _var)                           \

I added a temporary variable there to avoid expanding
the argument too many times.

       Arnd

Re: [PATCH 1/9] dm integrity: fix out-of-range warning

[Mikulas Patocka]

On Thu, 28 Mar 2024, Arnd Bergmann wrote:

> From: Arnd Bergmann <arnd@arndb.de>
> 
> Depending on the value of CONFIG_HZ, clang compares about a pointless
> comparison:
> 
> drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>                         if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {
> 
> As the check remains useful for other configurations, shut up the
> warning by adding a second type cast to uint64_t.
> 
> Fixes: 468dfca38b1a ("dm integrity: add a bitmap mode")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Mikulas Patocka <mpatocka@redhat.com>

> ---
>  drivers/md/dm-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 37b9f8f1ae1a..7f3dc8ee6ab8 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
>  		} else if (sscanf(opt_string, "sectors_per_bit:%llu%c", &llval, &dummy) == 1) {
>  			log2_sectors_per_bitmap_bit = !llval ? 0 : __ilog2_u64(llval);
>  		} else if (sscanf(opt_string, "bitmap_flush_interval:%u%c", &val, &dummy) == 1) {
> -			if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {
> +			if ((uint64_t)val >= (uint64_t)UINT_MAX * 1000 / HZ) {
>  				r = -EINVAL;
>  				ti->error = "Invalid bitmap_flush_interval argument";
>  				goto bad;
> -- 
> 2.39.2
> 

Re: [PATCH 1/9] dm integrity: fix out-of-range warning

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:31 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> Depending on the value of CONFIG_HZ, clang compares about a pointless
> comparison:
>
> drivers/md/dm-integrity.c:4085:12: error: result of comparison of constant 42949672950 with expression of type 'unsigned int' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>                         if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {
>
> As the check remains useful for other configurations, shut up the
> warning by adding a second type cast to uint64_t.

Yeah, nice.

>
> Fixes: 468dfca38b1a ("dm integrity: add a bitmap mode")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  drivers/md/dm-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 37b9f8f1ae1a..7f3dc8ee6ab8 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -4221,7 +4221,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
>                 } else if (sscanf(opt_string, "sectors_per_bit:%llu%c", &llval, &dummy) == 1) {
>                         log2_sectors_per_bitmap_bit = !llval ? 0 : __ilog2_u64(llval);
>                 } else if (sscanf(opt_string, "bitmap_flush_interval:%u%c", &val, &dummy) == 1) {
> -                       if (val >= (uint64_t)UINT_MAX * 1000 / HZ) {
> +                       if ((uint64_t)val >= (uint64_t)UINT_MAX * 1000 / HZ) {
>                                 r = -EINVAL;
>                                 ti->error = "Invalid bitmap_flush_interval argument";
>                                 goto bad;
> --
> 2.39.2
>

Re: [PATCH 6/9] nilfs2: fix out-of-range warning

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> clang-14 points out that v_size is always smaller than a 64KB
> page size if that is configured by the CPU architecture:

Is this only with clang-14?

>
> fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (argv->v_size > PAGE_SIZE)
>             ~~~~~~~~~~~~ ^ ~~~~~~~~~
>
> This is ok, so just shut up that warning with a cast.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

My question out of curiosity aside,

Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  fs/nilfs2/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index f1a01c191cf5..8be471ce4f19 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
>         if (argv->v_nmembs == 0)
>                 return 0;
>
> -       if (argv->v_size > PAGE_SIZE)
> +       if ((size_t)argv->v_size > PAGE_SIZE)
>                 return -EINVAL;
>
>         /*
> --
> 2.39.2
>

Re: [PATCH 8/9] mlx5: stop warning for 64KB pages

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> When building with 64KB pages, clang points out that xsk->chunk_size
> can never be PAGE_SIZE:

This is under W=1 right? Otherwise this is a mighty annoying warning.

>
> drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (xsk->chunk_size > PAGE_SIZE ||
>             ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~
>
> In older versions of this code, using PAGE_SIZE was the only
> possibility, so this would have never worked on 64KB page kernels,
> but the patch apparently did not address this case completely.
>
> As Maxim Mikityanskiy suggested, 64KB chunks are really not all that
> useful, so just shut up the warning by adding a cast.
>
> Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")
> Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/
> Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> index 06592b9f0424..9240cfe25d10 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
> @@ -28,8 +28,10 @@ bool mlx5e_validate_xsk_param(struct mlx5e_params *params,
>                               struct mlx5e_xsk_param *xsk,
>                               struct mlx5_core_dev *mdev)
>  {
> -       /* AF_XDP doesn't support frames larger than PAGE_SIZE. */
> -       if (xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {
> +       /* AF_XDP doesn't support frames larger than PAGE_SIZE,
> +        * and xsk->chunk_size is limited to 65535 bytes.
> +        */
> +       if ((size_t)xsk->chunk_size > PAGE_SIZE || xsk->chunk_size < MLX5E_MIN_XSK_CHUNK_SIZE) {
>                 mlx5_core_err(mdev, "XSK chunk size %u out of bounds [%u, %lu]\n", xsk->chunk_size,
>                               MLX5E_MIN_XSK_CHUNK_SIZE, PAGE_SIZE);
>                 return false;
> --
> 2.39.2
>

Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote:
> Add a cast to u32 in both cases, so it never warns about this.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  drivers/infiniband/core/uverbs_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
> index f80da6a67e24..e0cc3ddae71b 100644
> --- a/drivers/infiniband/core/uverbs_ioctl.c
> +++ b/drivers/infiniband/core/uverbs_ioctl.c
> @@ -90,7 +90,7 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,
>                 ALIGN(bundle_size + 256, sizeof(*pbundle->internal_buffer));
>
>         /* Do not want order-2 allocations for this. */
> -       WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
> +       WARN_ON_ONCE((u32)method_elm->bundle_size > PAGE_SIZE);
>  }
>
>  /**
> @@ -636,7 +636,7 @@ long ib_uverbs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         if (err)
>                 return -EFAULT;
>
> -       if (hdr.length > PAGE_SIZE ||
> +       if ((u32)hdr.length > PAGE_SIZE ||
>             hdr.length != struct_size(&hdr, attrs, hdr.num_attrs))
>                 return -EINVAL;
>
> --
> 2.39.2
>

Re: [PATCH 8/9] mlx5: stop warning for 64KB pages

[Arnd Bergmann]

On Thu, Mar 28, 2024, at 23:09, Justin Stitt wrote:
> On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> When building with 64KB pages, clang points out that xsk->chunk_size
>> can never be PAGE_SIZE:
>
> This is under W=1 right? Otherwise this is a mighty annoying warning.

At the moment yes. I'm fairly sure that I've covered all the
common cases with thousands of randconfig builds, so we should
be able to make it the default when this series is fully merged.

     Arnd

Re: [PATCH 4/9] kcov: avoid clang out-of-range warning

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:31 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The area_size is never larger than the maximum on 64-bit architectutes:
>
> kernel/kcov.c:634:29: error: result of comparison of constant 1152921504606846975 with expression of type '__u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>                 if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long))
>                     ~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The compiler can correctly optimize the check away and the code appears
> correct to me, so just add a cast to avoid the warning.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  kernel/kcov.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index f9ac2e9e460f..c3124f6d5536 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -627,7 +627,8 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                 mode = kcov_get_mode(remote_arg->trace_mode);
>                 if (mode < 0)
>                         return mode;
> -               if (remote_arg->area_size > LONG_MAX / sizeof(unsigned long))
> +               if ((unsigned long)remote_arg->area_size >
> +                   LONG_MAX / sizeof(unsigned long))
>                         return -EINVAL;
>                 kcov->mode = mode;
>                 t->kcov = kcov;
> --
> 2.39.2
>

Re: [PATCH 6/9] nilfs2: fix out-of-range warning

[Arnd Bergmann]

On Thu, Mar 28, 2024, at 23:04, Justin Stitt wrote:
> On Thu, Mar 28, 2024 at 7:32 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> clang-14 points out that v_size is always smaller than a 64KB
>> page size if that is configured by the CPU architecture:
>
> Is this only with clang-14?

No, it should be every version since then. I checked my
randconfig tree and found that I originally committed this
patch in September 2021 when I was first testing prerelease
clang-14 builds. I sent a couple of fixes for this class
of warnings then, but for some reason not this one.

      Arnd

Re: [PATCH 8/9] mlx5: stop warning for 64KB pages

[Tariq Toukan]

On 28/03/2024 16:30, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When building with 64KB pages, clang points out that xsk->chunk_size
> can never be PAGE_SIZE:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c:19:22: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>          if (xsk->chunk_size > PAGE_SIZE ||
>              ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~
> 
> In older versions of this code, using PAGE_SIZE was the only
> possibility, so this would have never worked on 64KB page kernels,
> but the patch apparently did not address this case completely.
> 
> As Maxim Mikityanskiy suggested, 64KB chunks are really not all that
> useful, so just shut up the warning by adding a cast.
> 
> Fixes: 282c0c798f8e ("net/mlx5e: Allow XSK frames smaller than a page")
> Link: https://lore.kernel.org/netdev/20211013150232.2942146-1-arnd@kernel.org/
> Link: https://lore.kernel.org/lkml/a7b27541-0ebb-4f2d-bd06-270a4d404613@app.fastmail.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for your patch.

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>

Re: [PATCH 2/9] libceph: avoid clang out-of-range warning

[Justin Stitt]

On Thu, Mar 28, 2024 at 7:31 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> clang-14 points out that on 64-bit architectures, a u32
> is never larger than constant based on SIZE_MAX:
>
> net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
>             ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
>             ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The code is correct anyway, so just shut up that warning.

OK.

>
> Fixes: 6f428df47dae ("libceph: pg_upmap[_items] infrastructure")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Justin Stitt <justinstitt@google.com>

> ---
>  fs/ceph/snap.c    | 2 +-
>  net/ceph/osdmap.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index c65f2b202b2b..521507ea8260 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,
>
>         /* alloc new snap context */
>         err = -ENOMEM;
> -       if (num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
> +       if ((size_t)num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
>                 goto fail;
>         snapc = ceph_create_snap_context(num, GFP_NOFS);
>         if (!snapc)
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 295098873861..8e7cb2fde6f1 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,
>         ceph_decode_32_safe(p, end, len, e_inval);
>         if (len == 0 && incremental)
>                 return NULL;    /* new_pg_temp: [] to remove */
> -       if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
> +       if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
>                 return ERR_PTR(-EINVAL);
>
>         ceph_decode_need(p, end, len * sizeof(u32), e_inval);
> @@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,
>         u32 len, i;
>
>         ceph_decode_32_safe(p, end, len, e_inval);
> -       if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
> +       if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
>                 return ERR_PTR(-EINVAL);
>
>         ceph_decode_need(p, end, 2 * len * sizeof(u32), e_inval);
> --
> 2.39.2
>

Re: [PATCH 3/9] rbd: avoid out-of-range warning

[Xiubo Li]

On 3/28/24 22:53, Alex Elder wrote:
> On 3/28/24 9:30 AM, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> clang-14 points out that the range check is always true on 64-bit
>> architectures since a u32 is not greater than the allowed size:
>>
>> drivers/block/rbd.c:6079:17: error: result of comparison of constant 
>> 2305843009213693948 with expression of type 'u32' (aka 'unsigned 
>> int') is always false 
>> [-Werror,-Wtautological-constant-out-of-range-compare]
> w
>>              ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This is harmless, so just change the type of the temporary to size_t
>> to shut up that warning.
>
> This fixes the warning, but then the now size_t value is passed
> to ceph_decode_32_safe(), which implies a different type conversion.
> That too is not harmful, but...
>
> Could we just cast the value in the comparison instead?
>
>   if ((size_t)snap_count > (SIZE_MAX - sizeof (struct ceph_snap_context))
>
> You could drop the space between sizeof and ( while
> you're at it (I always used the space back then).
>
Agree.

- Xiubo


> -Alex
>
>>
>> Fixes: bb23e37acb2a ("rbd: refactor rbd_header_from_disk()")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   drivers/block/rbd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 26ff5cd2bf0a..cb25ee513ada 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -6062,7 +6062,7 @@ static int rbd_dev_v2_snap_context(struct 
>> rbd_device *rbd_dev,
>>       void *p;
>>       void *end;
>>       u64 seq;
>> -    u32 snap_count;
>> +    size_t snap_count;
>>       struct ceph_snap_context *snapc;
>>       u32 i;
>
>

Re: [PATCH 2/9] libceph: avoid clang out-of-range warning

[Xiubo Li]

On 3/28/24 22:30, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> clang-14 points out that on 64-bit architectures, a u32
> is never larger than constant based on SIZE_MAX:
>
> net/ceph/osdmap.c:1425:10: error: result of comparison of constant 4611686018427387891 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>          if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
>              ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/ceph/osdmap.c:1608:10: error: result of comparison of constant 2305843009213693945 with expression of type 'u32' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>          if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
>              ~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The code is correct anyway, so just shut up that warning.
>
> Fixes: 6f428df47dae ("libceph: pg_upmap[_items] infrastructure")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   fs/ceph/snap.c    | 2 +-
>   net/ceph/osdmap.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index c65f2b202b2b..521507ea8260 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -374,7 +374,7 @@ static int build_snap_context(struct ceph_mds_client *mdsc,
>   
>   	/* alloc new snap context */
>   	err = -ENOMEM;
> -	if (num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
> +	if ((size_t)num > (SIZE_MAX - sizeof(*snapc)) / sizeof(u64))
>   		goto fail;
>   	snapc = ceph_create_snap_context(num, GFP_NOFS);
>   	if (!snapc)
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 295098873861..8e7cb2fde6f1 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -1438,7 +1438,7 @@ static struct ceph_pg_mapping *__decode_pg_temp(void **p, void *end,
>   	ceph_decode_32_safe(p, end, len, e_inval);
>   	if (len == 0 && incremental)
>   		return NULL;	/* new_pg_temp: [] to remove */
> -	if (len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
> +	if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / sizeof(u32))
>   		return ERR_PTR(-EINVAL);
>   
>   	ceph_decode_need(p, end, len * sizeof(u32), e_inval);
> @@ -1621,7 +1621,7 @@ static struct ceph_pg_mapping *__decode_pg_upmap_items(void **p, void *end,
>   	u32 len, i;
>   
>   	ceph_decode_32_safe(p, end, len, e_inval);
> -	if (len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
> +	if ((size_t)len > (SIZE_MAX - sizeof(*pg)) / (2 * sizeof(u32)))
>   		return ERR_PTR(-EINVAL);
>   
>   	ceph_decode_need(p, end, 2 * len * sizeof(u32), e_inval);


Reviewed-by: Xiubo Li <xiubli@redhat.com>

Thanks

- Xiubo

Re: [PATCH 6/9] nilfs2: fix out-of-range warning

[Ryusuke Konishi]

On Thu, Mar 28, 2024 at 11:32 PM Arnd Bergmann wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> clang-14 points out that v_size is always smaller than a 64KB
> page size if that is configured by the CPU architecture:
>
> fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (argv->v_size > PAGE_SIZE)
>             ~~~~~~~~~~~~ ^ ~~~~~~~~~
>
> This is ok, so just shut up that warning with a cast.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/nilfs2/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index f1a01c191cf5..8be471ce4f19 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -60,7 +60,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
>         if (argv->v_nmembs == 0)
>                 return 0;
>
> -       if (argv->v_size > PAGE_SIZE)
> +       if ((size_t)argv->v_size > PAGE_SIZE)
>                 return -EINVAL;
>
>         /*
> --
> 2.39.2
>

Andrew, could you please apply this to the -mm tree along with the
following tags and Justin's reviewed-by tag?

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Fixes: 3358b4aaa84f ("nilfs2: fix problems of memory allocation in ioctl")

Thank you, Arnd.
I didn't notice this warning existed depending on the environment.

Ryusuke Konishi

Re: [PATCH 0/9] address remaining -Wtautological-constant-out-of-range-compare

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 28 Mar 2024 15:30:38 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The warning option was introduced a few years ago but left disabled
> by default. All of the actual bugs that this has found have been
> fixed in the meantime, and this series should address the remaining
> false-positives, as tested on arm/arm64/x86 randconfigs as well as
> allmodconfig builds for all architectures supported by clang.
> 
> [...]

Here is the summary with links:
  - [2/9] libceph: avoid clang out-of-range warning
    (no matching commit)
  - [5/9] ipv4: tcp_output: avoid warning about NET_ADD_STATS
    (no matching commit)
  - [8/9] mlx5: stop warning for 64KB pages
    https://git.kernel.org/netdev/net-next/c/a5535e533694

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: (subset) [PATCH 6/9] nilfs2: fix out-of-range warning

[Christian Brauner]

On Thu, 28 Mar 2024 15:30:44 +0100, Arnd Bergmann wrote:
> clang-14 points out that v_size is always smaller than a 64KB
> page size if that is configured by the CPU architecture:
> 
> fs/nilfs2/ioctl.c:63:19: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (argv->v_size > PAGE_SIZE)
>             ~~~~~~~~~~~~ ^ ~~~~~~~~~
> 
> [...]

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[6/9] nilfs2: fix out-of-range warning
      https://git.kernel.org/vfs/vfs/c/d20180d5dd10

Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings

[Jason Gunthorpe]

On Thu, Mar 28, 2024 at 03:30:45PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang warns for comparisons that are always true, which is the case
> for these two page size checks on architectures with 64KB pages:
> 
> drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE'
>         int __ret_warn_on = !!(condition);                      \
>                                ^~~~~~~~~
> drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (hdr.length > PAGE_SIZE ||
>             ~~~~~~~~~~ ^ ~~~~~~~~~
> 
> Add a cast to u32 in both cases, so it never warns about this.

But doesn't that hurt the codegen?

Jason

Re: [PATCH 7/9] infiniband: uverbs: avoid out-of-range warnings

[Arnd Bergmann]

On Wed, Apr 3, 2024, at 17:45, Jason Gunthorpe wrote:
> On Thu, Mar 28, 2024 at 03:30:45PM +0100, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> clang warns for comparisons that are always true, which is the case
>> for these two page size checks on architectures with 64KB pages:
>> 
>> drivers/infiniband/core/uverbs_ioctl.c:90:39: error: result of comparison of constant 65536 with expression of type 'u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>>         WARN_ON_ONCE(method_elm->bundle_size > PAGE_SIZE);
>>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
>> include/asm-generic/bug.h:104:25: note: expanded from macro 'WARN_ON_ONCE'
>>         int __ret_warn_on = !!(condition);                      \
>>                                ^~~~~~~~~
>> drivers/infiniband/core/uverbs_ioctl.c:621:17: error: result of comparison of constant 65536 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>>         if (hdr.length > PAGE_SIZE ||
>>             ~~~~~~~~~~ ^ ~~~~~~~~~
>> 
>> Add a cast to u32 in both cases, so it never warns about this.
>
> But doesn't that hurt the codegen?

I just double-checked in the compiler explorer to confirm that
this works as I expected: both gcc and clang are still able
to optimize out the comparison for 64K pages, but clang no
longer complains after my change that this is an obvious
case.

I also see that gcc still produces a -Wtype-limits warning, but that
likely has to stay disabled because it produces too much output
elsewhere and I don't see an easy way to shut it up.

     Arnd