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

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 [off-list ref]

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 (local)

[ ... ]

 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