Re: [PATCH v4 1/4] PCI: of: avoid allocations in of_pci_prop_compatible()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The local variable `bufp` is incorrectly declared as `char` instead of `char *`. This truncates the stack array pointer `buf` to an 8-bit integer, leading to severe memory corruption and an immediate kernel panic during runtime.
- [Low] The local variable `i` is declared but never used after the patch.
--

commit 0153310b625600febac7ea5aec9fbd9871d0af03
Author: Alex Elder <elder@riscstar.com>

PCI: of: avoid allocations in of_pci_prop_compatible()

This patch replaces dynamic allocations for three compatible strings with a
single stack-allocated buffer. This avoids subsequent kernel crashes where
allocation failures would unconditionally trigger strlen() during device tree
properties initialization.

Link: https://lore.kernel.org/sashiko-reviews/a647bd56-7dc8-4fec-9d96-834622054cdf@riscstar.com

> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
> --- a/drivers/pci/of_property.c
> +++ b/drivers/pci/of_property.c
[ ... ]
>  static int of_pci_prop_compatible(struct pci_dev *pdev,
>  				  struct of_changeset *ocs,
>  				  struct device_node *np)
>  {
>  	const char *compat_strs[PROP_COMPAT_NUM] = { 0 };
> +	char buf[PROP_COMPAT_NUM * PROP_SIZE] = { };
> +	char bufp = buf;

[Severity: High]
Does this incorrectly declare bufp as a single char instead of a pointer?
Assigning the array address buf to an 8-bit char truncates the memory address.

>  	int i, ret;

[Severity: Low]
Is the variable i still needed here? The loop that used it to free the
allocated strings was removed by this patch, which will leave this as an
unused variable and cause a compiler warning.

> -	compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] =
> -		kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device);
> -	compat_strs[PROP_COMPAT_PCICLASS_CCSSPP] =
> -		kasprintf(GFP_KERNEL, "pciclass,%06x", pdev->class);
> -	compat_strs[PROP_COMPAT_PCICLASS_CCSS] =
> -		kasprintf(GFP_KERNEL, "pciclass,%04x", pdev->class >> 8);
> +	ret = snprintf(bufp, PROP_SIZE, "pci%x,%x", pdev->vendor, pdev->device);

[Severity: High]
Will this snprintf call trigger a kernel panic upon dereference? Because bufp
is truncated to an 8-bit integer, passing it as the output buffer pointer to
snprintf() will result in an out-of-bounds memory access (unmapped low-memory
dereference) when evaluated during system boot or PCI device enumeration.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904134607.1856121-1-elder@riscstar.com?part=1