Re: [PATCH v11 23/31] dax/bus: Factor out dev dax resize logic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Integer overflow in __dev_dax_resize causes >2GB allocations to be treated as errors, leaking DAX resources on 32-bit systems.
--

commit abac505d7b066ca0edac0e8d7df59c037cc29e89
Author: Ira Weiny <iweiny@kernel.org>

dax/bus: Factor out dev dax resize logic

This commit factors out the device dax resize logic to prepare for dynamic
capacity regions. It introduces a helper function that allocates ranges
using a specified parent resource, returning the successfully allocated size.

> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 95683dc8fcd0c..ffa6b303fc9bd 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
[ ... ]
> +static ssize_t dev_dax_resize_static(struct resource *parent,
> +				     struct dev_dax *dev_dax,
> +				     resource_size_t to_alloc)
>  {
>  	struct resource *res, *first;
>  	int rc;
>  
> +	first = parent->child;
> +	if (!first) {
> +		rc = alloc_dev_dax_range(parent, dev_dax,
> +					   parent->start, to_alloc, NULL);
> +		if (rc)
> +			return rc;
> +		return to_alloc;
> +	}
>  
>  	for (res = first; res; res = res->sibling) {
>  		struct resource *next = res->sibling;
> +		resource_size_t alloc;
>  
>  		/* space at the beginning of the region */
> +		if (res == first && res->start > parent->start) {
> +			alloc = min(res->start - parent->start, to_alloc);
> +			rc = alloc_dev_dax_range(parent, dev_dax,
> +						 parent->start, alloc, NULL);
> +			if (rc)
> +				return rc;
> +			return alloc;

[Severity: High]
On 32-bit systems where ssize_t is a 32-bit signed integer and
resource_size_t can be an unsigned 64-bit value, does returning a 2GB or
larger alloc size cause an integer overflow?

If alloc is >= 2GB, returning it as ssize_t could wrap to a negative number
or zero if truncated from a larger type.

[ ... ]
> +static ssize_t dev_dax_resize(struct dax_region *dax_region,
> +		struct dev_dax *dev_dax, resource_size_t size)
> +{
[ ... ]
> +retry:
> +	alloc = dev_dax_resize_static(&dax_region->res, dev_dax, to_alloc);
> +	if (alloc < 0)
> +		return alloc;

[Severity: High]
If a large allocation overflows into a negative ssize_t, wouldn't this caller
evaluate alloc < 0 as true and mistakenly treat the successful allocation as
an error?

Because the successfully mapped chunk was already added to dev_dax->ranges in
dev_dax_resize_static(), aborting the resize here without unwinding the newly
allocated range seems like it would permanently leak the DAX space and leave
the region in an inconsistent state.

> +	if (alloc == 0)
> +		return -ENOSPC;
>  	to_alloc -= alloc;
>  	if (to_alloc)
>  		goto retry;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625112638.550691-1-anisa.su@samsung.com?part=23