Re: [PATCH 1/2] xfs_repair: do not allow cache growth to overflow

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Aug 24, 2026 at 02:58:59PM -0500, user.mail wrote:
> Nothing stops cache_expand() from growing c_maxcount (via *2)
> repeatedly until it overflows.
> 
> Add a bounds check here and return failure if for any reason we try to
> grow too much.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: user.mail <sandeen@redhat.com>
> ---
>  libxfs/cache.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/libxfs/cache.c b/libxfs/cache.c
> index d5d9ba56..f8be9b89 100644
> --- a/libxfs/cache.c
> +++ b/libxfs/cache.c
> @@ -79,16 +79,23 @@ cache_init(
>  	return cache;
>  }
>  
> -static void
> +static int
>  cache_expand(
>  	struct cache *		cache)
>  {
> +	int			success = 1;
>  	pthread_mutex_lock(&cache->c_mutex);
> +	if (cache->c_maxcount <= UINT_MAX/2) {
>  #ifdef CACHE_DEBUG
> -	fprintf(stderr, "doubling cache size to %u\n", 2 * cache->c_maxcount);
> +		fprintf(stderr, "doubling cache size to %u\n",
> +			2 * cache->c_maxcount);
>  #endif
> -	cache->c_maxcount *= 2;
> +		cache->c_maxcount *= 2;

I agree that it's useful not to overflow c_maxcount, but how did you end
up with that many cached buffers?  Or is this LLM-inspired bugfixes?

Either way I'm ok with this part, so...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

> +	} else
> +		success = 0;
>  	pthread_mutex_unlock(&cache->c_mutex);
> +
> +	return success;
>  }
>  
>  void
> @@ -468,7 +475,8 @@ next_object:
>  		 */
>  		if (priority > CACHE_MAX_PRIORITY) {
>  			priority = 0;
> -			cache_expand(cache);
> +			if (!cache_expand(cache))
> +				goto fail;
>  		}
>  	}
>  
> @@ -488,6 +496,10 @@ next_object:
>  
>  	*nodep = node;
>  	return 1;
> +
> +fail:
> +	*nodep = NULL;
> +	return -1;

...but a general comment about this patch and the next one: it seems
reasonable to make cache_node_get return a null nodep and -1 to indicate
error, but AFAICT the only caller is __cache_lookup (aka the buffer
cache), which doesn't check the return value at all, only the value of
*nodep.

I don't see a change in this series to change __cache_lookup.  Did that
get lost somewhere?

--D

>  }
>  
>  void
> -- 
> 2.55.0
> 
>