Received-SPF: pass (google.com: domain of linux-kernel+bounces-112450-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 23 Mar 2024 11:45:02 -0700
From: "Darrick J. Wong" <djwong@kernel.org>
To: Andrey Shumilin <shum.sdl@nppct.ru>
Cc: Chandan Babu R <chandan.babu@oracle.com>,
	Dave Chinner <dchinner@redhat.com>, linux-xfs@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>,
	lvc-project@linuxtesting.org, khoroshilov@ispras.ru,
	ykarpov@ispras.ru, vmerzlyakov@ispras.ru, vefanov@ispras.ru
Subject: Re: [PATCH 2/2] xfs_refcount: Preventing integer overflow
Message-ID: <20240323184502.GA6390@frogsfrogsfrogs>
References: <20240323062603.665572-1-shum.sdl@nppct.ru>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240323062603.665572-1-shum.sdl@nppct.ru>

On Sat, Mar 23, 2024 at 09:26:03AM +0300, Andrey Shumilin wrote:
> Multiplying variables can overflow the "overhead" variable.
> To fix this, the variable type has been increased.
> Next, a subtraction operation occurs with it,
> but before that it is checked.

Under what circumstances will pre-multiplication @overhead have a large
enough value to overflow?  The blocksize cannot be larger than 2^16, and
full splits of three btrees should never require anywhere close to 2^16
blocks, right?  Did your analysis tool find a scenario where this
actually happens?

--D

> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
> ---
>  fs/xfs/libxfs/xfs_refcount.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 511c912d515c..cbf07552eaff 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -1070,7 +1070,7 @@ static bool
>  xfs_refcount_still_have_space(
>  	struct xfs_btree_cur		*cur)
>  {
> -	unsigned long			overhead;
> +	unsigned long long		overhead;
>  
>  	/*
>  	 * Worst case estimate: full splits of the free space and rmap btrees
> -- 
> 2.30.2
> 
>