linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	Richard Fung <richardfung@google.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH] fuse: fs-verity: aoid out-of-range comparison
Date: Thu, 1 Aug 2024 01:09:53 +0000	[thread overview]
Message-ID: <20240801010953.GA1835661@google.com> (raw)
In-Reply-To: <20240730142802.1082627-1-arnd@kernel.org>

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

  parent reply	other threads:[~2024-08-01  1:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2024-08-01 17:45   ` Richard Fung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240801010953.GA1835661@google.com \
    --to=ebiggers@kernel.org \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=justinstitt@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=miklos@szeredi.hu \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=richardfung@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).