Re: [PATCH v2] PCI: Reject negative resource alignment order

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3635 bytes --]

On Mon, 9 Nov 2026, shao.mingyin@zte.com.cn wrote:

> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> pci_specified_resource_alignment() parses the pci=resource_alignment=
> boot parameter and rejects only orders above 63.  A negative order, such
> as "pci=resource_alignment=-5@...", passes that check and later reaches
> "align = 1ULL << align_order" with a negative shift count, which is
> undefined behavior; on x86 the 64-bit shift count is masked to six bits,
> silently producing a 2^59-byte alignment that mis-aligns (or fails to
> align) the affected IOV resources.
> 
> align_order is a bit order, so parse it with %u.  vsscanf() does not
> accept a sign for %u, so a negative order no longer converts: the
> parameter is reported as invalid and no alignment is applied.  Orders
> above 63 are still rejected and fall back to the page-size alignment.
> 
> Fixes: 6534aac198b5 ("PCI: Bounds-check command-line resource alignment requests")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> This supersedes 1/5 of the v1 series; 2/5-5/5 are unchanged.
> 
> v2:
>   - parse the order with %u instead of %d, as Ilpo Järvinen suggested; a
>     bit order is unsigned, and this also removes the explicit < 0 test
>     added by v1
>   - pr_err() prints the order with %u as well
>   - note the resulting behaviour: vsscanf() does not accept a sign for
>     %u, so a negative order fails to parse instead of being clamped
> v1: https://lore.kernel.org/all/20261107002311316z2o2PFtFGoXh-VbqXtTGg@zte.com.cn/
> 
> Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the
> pci=resource_alignment= boot parameter and a QEMU e1000 NIC; no extra
> hardware is needed.
> 
> Booting with pci=resource_alignment=-5@pci:8086:100e, the unpatched
> kernel parses -5 and "align = 1ULL << align_order" shifts by -5 (masked
> to 2^59 on x86-64); the e1000 BAR is resized to 2^59 bytes, which cannot
> be assigned, and the e1000 fails to probe.  With this patch "-5" does not
> convert and the parameter is rejected:
> 
>   unpatched                            patched
>   requesting alignment to              PCI: Can't parse resource_alignment
>   0x800000000000000                    parameter: -5@pci:8086:100e
>   e1000 probe failed, no eth0          e1000 eth0 probes
> 
>  drivers/pci/pci.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index bca3a5d84cd9..e3f701ff9881 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6585,7 +6585,8 @@ static DEFINE_SPINLOCK(resource_alignment_lock);
>  static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>  							bool *resize)
>  {
> -	int align_order, count;
> +	unsigned int align_order;
> +	int count;
>  	resource_size_t align = pcibios_default_alignment();
>  	const char *p;
>  	int ret;

I'd have preferred having count and ret on the same line.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

--
 i.

> @@ -6602,11 +6603,11 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>  
>  	while (*p) {
>  		count = 0;
> -		if (sscanf(p, "%d%n", &align_order, &count) == 1 &&
> +		if (sscanf(p, "%u%n", &align_order, &count) == 1 &&
>  		    p[count] == '@') {
>  			p += count + 1;
>  			if (align_order > 63) {
> -				pr_err("PCI: Invalid requested alignment (order %d)\n",
> +				pr_err("PCI: Invalid requested alignment (order %u)\n",
>  				       align_order);
>  				align_order = PAGE_SHIFT;
>  			}
>