Re: [PATCH v2 5/7] PCI: Place resources to either edge of the window

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: 64-bit truncation in pci_max_natural_size() on 32-bit archs
--

commit a3daecf300277b71086888c6fc930ab2932e9d1b
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Place resources to either edge of the window

This introduces a new greedy algorithm for PCI resource assignment that
is aware of window edges. It aims to avoid fragmenting the free space
when placing composite resources like bridge windows and VF BARs.

> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 2ca3784aa323f..4c73bf6e0f706 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
[ ... ]
> @@ -262,16 +265,89 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
>  	return resource_alignment(res);
>  }
>  
> +static resource_size_t pci_max_natural_size(const struct resource *res,
> +					    resource_size_t *max_align)
> +{
> +	resource_size_t size = resource_size(res);
> +	resource_size_t powof2, natural_start;
> +
> +	*max_align = 1;
> +	if (!size)
> +		return 0;
> +
> +	powof2 = rounddown_pow_of_two(size);

[Severity: High]
Does pci_max_natural_size() in drivers/pci/setup-res.c silently truncate the
64-bit size to 32 bits on 32-bit architectures?

If CONFIG_PHYS_ADDR_T_64BIT=y is enabled on a 32-bit architecture and a
resource is >= 4GB, rounddown_pow_of_two() will receive a 0-truncated value
since it expects an unsigned long. Can this result in a shift by -1
(undefined behavior) and cause incorrect BAR size and alignment calculations?

> +	natural_start = ALIGN(res->start, powof2);
> +	/* end = ~0 first overflows, then -1 brings it back */
> +	if (natural_start >= ALIGN_DOWN(res->end + 1, powof2) - 1) {
> +		powof2 = max(powof2 / 2, 1U);
> +		natural_start = ALIGN(res->start, powof2);
> +	}
> +
> +	if (natural_start) {
> +		*max_align <<= __ffs(natural_start);

[Severity: High]
Can pci_max_natural_size() in drivers/pci/setup-res.c invoke undefined
behavior on 32-bit architectures if the start address is aligned to a 4GB
boundary?

The __ffs() function expects an unsigned long. If the 64-bit natural_start
has its lower 32 bits as zero, this implicitly casts to a 32-bit zero, which
triggers undefined behavior in __builtin_ctzl(0).

> +	} else {
> +		/*
> +		 * Zero address has infinite alignment, return the largest
> +		 * representable number even if it's not a power of two.
> +		 */
> +		*max_align = RESOURCE_SIZE_MAX;
> +	}
> +
> +	return powof2;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=5