2024-03-23 06:26:32

by Andrey Shumilin

[permalink] [raw]
Subject: [PATCH 2/2] xfs_refcount: Preventing integer overflow

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.

Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Andrey Shumilin <[email protected]>
---
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



2024-03-23 18:45:11

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH 2/2] xfs_refcount: Preventing integer overflow

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 <[email protected]>
> ---
> 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
>
>