Building the Linux kernel with Clang and LLVM
 help / color / mirror / Atom feed
* [PATCH] fuse: fs-verity: aoid out-of-range comparison
@ 2024-07-30 14:27 Arnd Bergmann
  2024-08-01  0:35 ` Nathan Chancellor
  2024-08-01  1:09 ` Eric Biggers
  0 siblings, 2 replies; 7+ messages in thread
From: Arnd Bergmann @ 2024-07-30 14:27 UTC (permalink / raw)
  To: Miklos Szeredi, Richard Fung, Eric Biggers
  Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, linux-fsdevel, linux-kernel, llvm

From: Arnd Bergmann <arnd@arndb.de>

clang points out that comparing the 16-bit size of the digest against
SIZE_MAX is not a helpful comparison:

fs/fuse/ioctl.c:130:18: error: result of comparison of constant 18446744073709551611 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
        if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
            ~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This either means tha tthe check can be removed entirely, or that the
intended comparison was for the 16-bit range. Assuming the latter was
intended, compare against U16_MAX instead.

Fixes: 9fe2a036a23c ("fuse: Add initial support for fs-verity")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/fuse/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index 572ce8a82ceb..5711d86c549d 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -127,7 +127,7 @@ static int fuse_setup_measure_verity(unsigned long arg, struct iovec *iov)
 	if (copy_from_user(&digest_size, &uarg->digest_size, sizeof(digest_size)))
 		return -EFAULT;
 
-	if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
+	if (digest_size > U16_MAX - sizeof(struct fsverity_digest))
 		return -EINVAL;
 
 	iov->iov_len = sizeof(struct fsverity_digest) + digest_size;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2024-07-30 14:27 [PATCH] fuse: fs-verity: aoid out-of-range comparison Arnd Bergmann
@ 2024-08-01  0:35 ` Nathan Chancellor
  2024-08-01  1:09 ` Eric Biggers
  1 sibling, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2024-08-01  0:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Miklos Szeredi, Richard Fung, Eric Biggers, Arnd Bergmann,
	Nick Desaulniers, Bill Wendling, Justin Stitt, linux-fsdevel,
	linux-kernel, llvm

On Tue, Jul 30, 2024 at 04:27:52PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang points out that comparing the 16-bit size of the digest against
> SIZE_MAX is not a helpful comparison:
> 
> fs/fuse/ioctl.c:130:18: error: result of comparison of constant 18446744073709551611 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
>             ~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This either means tha tthe check can be removed entirely, or that the
> intended comparison was for the 16-bit range. Assuming the latter was
> intended, compare against U16_MAX instead.

Presumably this check was added because of the addition in the
assignment to iov_len, which is size_t, but I don't see how that
expression could realistically overflow? It seems like this whole check
could just be removed?

> Fixes: 9fe2a036a23c ("fuse: Add initial support for fs-verity")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/fuse/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index 572ce8a82ceb..5711d86c549d 100644
> --- a/fs/fuse/ioctl.c
> +++ b/fs/fuse/ioctl.c
> @@ -127,7 +127,7 @@ static int fuse_setup_measure_verity(unsigned long arg, struct iovec *iov)
>  	if (copy_from_user(&digest_size, &uarg->digest_size, sizeof(digest_size)))
>  		return -EFAULT;
>  
> -	if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
> +	if (digest_size > U16_MAX - sizeof(struct fsverity_digest))
>  		return -EINVAL;
>  
>  	iov->iov_len = sizeof(struct fsverity_digest) + digest_size;
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2024-07-30 14:27 [PATCH] fuse: fs-verity: aoid out-of-range comparison Arnd Bergmann
  2024-08-01  0:35 ` Nathan Chancellor
@ 2024-08-01  1:09 ` Eric Biggers
  2024-08-01 17:45   ` Richard Fung
  1 sibling, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2024-08-01  1:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Miklos Szeredi, Richard Fung, Arnd Bergmann, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, linux-fsdevel,
	linux-kernel, llvm

On Tue, Jul 30, 2024 at 04:27:52PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang points out that comparing the 16-bit size of the digest against
> SIZE_MAX is not a helpful comparison:
> 
> fs/fuse/ioctl.c:130:18: error: result of comparison of constant 18446744073709551611 with expression of type '__u16' (aka 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
>         if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
>             ~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This either means tha tthe check can be removed entirely, or that the
> intended comparison was for the 16-bit range. Assuming the latter was
> intended, compare against U16_MAX instead.
> 
> Fixes: 9fe2a036a23c ("fuse: Add initial support for fs-verity")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/fuse/ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index 572ce8a82ceb..5711d86c549d 100644
> --- a/fs/fuse/ioctl.c
> +++ b/fs/fuse/ioctl.c
> @@ -127,7 +127,7 @@ static int fuse_setup_measure_verity(unsigned long arg, struct iovec *iov)
>  	if (copy_from_user(&digest_size, &uarg->digest_size, sizeof(digest_size)))
>  		return -EFAULT;
>  
> -	if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
> +	if (digest_size > U16_MAX - sizeof(struct fsverity_digest))
>  		return -EINVAL;
>  
>  	iov->iov_len = sizeof(struct fsverity_digest) + digest_size;

I think this was just defensive coding to ensure that the assignment to iov_len
can't overflow regardless of the type of digest_size.  You can remove the check
if you want to, though isn't the tautological comparison warning disabled by the
kernel build system anyway?  Anyway, it does not make sense to use U16_MAX here.

- Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2024-08-01  1:09 ` Eric Biggers
@ 2024-08-01 17:45   ` Richard Fung
  2026-06-04 17:41     ` Andy Shevchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Fung @ 2024-08-01 17:45 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Arnd Bergmann, Miklos Szeredi, Arnd Bergmann, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, linux-fsdevel,
	linux-kernel, llvm

> I think this was just defensive coding to ensure that the assignment to iov_len can't overflow regardless of the type of digest_size

I agree with this. Removing it sounds good to me

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2024-08-01 17:45   ` Richard Fung
@ 2026-06-04 17:41     ` Andy Shevchenko
  2026-06-04 19:48       ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-06-04 17:41 UTC (permalink / raw)
  To: Richard Fung
  Cc: Eric Biggers, Arnd Bergmann, Miklos Szeredi, Arnd Bergmann,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	linux-fsdevel, linux-kernel, llvm

On Thu, Aug 01, 2024 at 10:45:01AM -0700, Richard Fung wrote:
> > I think this was just defensive coding to ensure that the assignment to
> > iov_len can't overflow regardless of the type of digest_size
> 
> I agree with this. Removing it sounds good to me

Where we are with this? I have just stumbled over this issue yesterday when
tried to have `make W=1` clean build of fs/.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2026-06-04 17:41     ` Andy Shevchenko
@ 2026-06-04 19:48       ` Arnd Bergmann
  2026-06-05 20:43         ` Arnd Bergmann
  0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2026-06-04 19:48 UTC (permalink / raw)
  To: Andy Shevchenko, Richard Fung
  Cc: Eric Biggers, Arnd Bergmann, Miklos Szeredi, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, linux-fsdevel,
	linux-kernel, llvm

On Thu, Jun 4, 2026, at 19:41, Andy Shevchenko wrote:
> On Thu, Aug 01, 2024 at 10:45:01AM -0700, Richard Fung wrote:
>> > I think this was just defensive coding to ensure that the assignment to
>> > iov_len can't overflow regardless of the type of digest_size
>> 
>> I agree with this. Removing it sounds good to me
>
> Where we are with this? I have just stumbled over this issue yesterday when
> tried to have `make W=1` clean build of fs/.

I have nine of these in my randconfig build tree tree at the moment, and
with my patches I get a clean build with the warning enabled, I sent
these patches when I first came across them, usually after a code change,
though some of them may be old:

    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]

    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]

    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]

    drivers/clk/actions/owl-pll.c:182:2: error: result of comparison of constant 2000 with expression of type 'u8' (aka 'unsi
gned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]

    arch/arm/mach-omap2/wd_timer.c:89:3: error: result of comparison of constant 2000 with expression of type 'u8' (aka 'unsi
gned char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]

    fs/fuse/ioctl.c:130:18: error: result of comparison of constant 18446744073709551611 with expression of type '__u16' (aka
 'unsigned short') is always false [-Werror,-Wtautological-constant-out-of-range-compare]

    drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c:1630:32: 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]

    arch/riscv/errata/sifive/errata.c:29:14: error: result of comparison of constant 9223372036854775815 with expression of t
ype 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
    arch/riscv/errata/sifive/errata.c:42:14: error: result of comparison of constant 9223372036854775815 with expression of t
ype 'unsigned long' is always true [-Werror,-Wtautological-constant-out-of-range-compare]

    drivers/gpu/drm/nouveau/nvkm/subdev/fb/gb202.c:17:37: error: result of comparison of constant 4503599627370495 with expre
ssion of type 'dma_addr_t' (aka 'unsigned int') is always false [-Werror,-Wtautological-constant-out-of-range-compare]

If you think there is a chance to enable it by default soon, I can
try to see which ones are still required and send them again.

       Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
  2026-06-04 19:48       ` Arnd Bergmann
@ 2026-06-05 20:43         ` Arnd Bergmann
  0 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2026-06-05 20:43 UTC (permalink / raw)
  To: Arnd Bergmann, Andy Shevchenko, Richard Fung
  Cc: Eric Biggers, Miklos Szeredi, Nathan Chancellor, Nick Desaulniers,
	Bill Wendling, Justin Stitt, linux-fsdevel, linux-kernel, llvm

On Thu, Jun 4, 2026, at 21:48, Arnd Bergmann wrote:
>
> I have nine of these in my randconfig build tree tree at the moment, and
> with my patches I get a clean build with the warning enabled, I sent
> these patches when I first came across them, usually after a code change,
> though some of them may be old:

I've pushed the remaining fixes to

https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=Wtautological-constant

I have not yet gone back to see if there were comments that I
need to address before resending.

       Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-05 20:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-30 14:27 [PATCH] fuse: fs-verity: aoid out-of-range comparison Arnd Bergmann
2024-08-01  0:35 ` Nathan Chancellor
2024-08-01  1:09 ` Eric Biggers
2024-08-01 17:45   ` Richard Fung
2026-06-04 17:41     ` Andy Shevchenko
2026-06-04 19:48       ` Arnd Bergmann
2026-06-05 20:43         ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox