Re: [PATCH v3 3/8] PCI: Honor alignment overrides

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: uninitialized variable `user_aligned` causes random bypass of resource alignment
--

commit dc4a5f31af386d2a1de50debe4823977860a1cb9
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Subject: PCI: Honor alignment overrides

This commit introduces struct pci_resreq_data to hold device and user-given
alignment. It allows pci_align_resource() to honor user-given alignment by
skipping remainder movement when a user alignment is provided.

Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/

[ ... ]
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..ea7d758712691 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
[ ... ]
> @@ -1217,12 +1217,40 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
>  /* Architecture-specific versions may override this (weak) */
>  char *pcibios_setup(char *str);
>  
> -/* Used only when drivers/pci/setup.c is used */
> +/* Resource assignment request handling */
> +/**
> + * struct pci_resreq_data - Data passed to pcibios_align_resource()
> + * @dev: PCI device
> + * @user_align: User provided alignment override (pci=resource_alignment param)
> + */
> +struct pci_resreq_data {
> +	struct pci_dev *dev;
> +	resource_size_t user_align;
> +};
> +
> +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> +						 bool *resize);
> +/*
> + * TODO: Improve PCI resource assignment interface so that callers outside PCI
> + * core do not need to know about this detail.
> + */
> +static inline void pci_init_pci_resreq_data(struct pci_resreq_data *rr,
> +					    struct pci_dev *dev)
> +{
> +	bool user_aligned;
> +
> +	memset(rr, 0, sizeof(*rr));
> +	rr->dev = dev;
> +	rr->user_align = pci_specified_resource_alignment(dev, &user_aligned);

[Severity: High]
Does this leave user_aligned uninitialized on the default execution path?

When pci_init_pci_resreq_data() calls pci_specified_resource_alignment(),
the output parameter is not always written to. For instance, if there
are no boot parameters, pci_specified_resource_alignment() exits early:

drivers/pci/pci.c:pci_specified_resource_alignment() {
    ...
	p = resource_alignment_param;
	if (!p || !*p)
		goto out;
    ...
out:
	spin_unlock(&resource_alignment_lock);
	return align;
}

This leaves user_aligned with uninitialized stack memory. If this memory
happens to be non-zero, it can cause the resource alignment logic to be
randomly bypassed later in pci_align_resource(), leading to potential PCI
resource exhaustion or allocation failures on architectures with a non-zero
default alignment.

Could user_aligned be initialized to false before being passed?

> +	if (!user_aligned)
> +		rr->user_align = 0;
> +}
> +
>  resource_size_t pcibios_align_resource(void *data, const struct resource *res,

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