Re: [PATCH v3 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags()

[Alex Elder <elder@riscstar.com>]

On 8/31/26 8:20 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Potential NULL pointer dereference in of_pci_prop_compatible if kasprintf fails.

This issue was reported before, and I responded that I would fix it.
I *do* have a fix, but I have not yet posted it for review (waiting
for the end of the merge window).  I will post it soon.

					-Alex

> --
> 
> commit 036872e87dc6e9201c6f86349afb91815a638d1e
> Author: Alex Elder <elder@riscstar.com>
> 
> PCI: of: don't zero flags in of_pci_get_addr_flags()
> 
> The flags variable whose address is passed to of_pci_get_addr_flags()
> is zeroed before assigning a value to it.  Skip the zeroing and just
> assign it instead.
> 
>> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
>> index 505226b876c56..22fea1905a080 100644
>> --- a/drivers/pci/of_property.c
>> +++ b/drivers/pci/of_property.c
>> @@ -82,12 +82,10 @@ static int of_pci_get_addr_flags(const struct resource *res, u32 *flags)
>>   	else
>>   		return -EINVAL;
>>   
>> -	*flags = 0;
>> +	*flags = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, ss);
>>   	if (res->flags & IORESOURCE_PREFETCH)
>>   		*flags |= OF_PCI_ADDR_FIELD_PREFETCH;
>>   
>> -	*flags |= FIELD_PREP(OF_PCI_ADDR_FIELD_SS, ss);
>> -
>>   	return 0;
>>   }
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential null pointer
> dereference in this file under low memory conditions.
> 
> Looking at of_pci_prop_compatible(), kasprintf() can fail and return a null
> pointer:
> 
> drivers/pci/of_property.c:of_pci_prop_compatible() {
> 	...
> 	compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] =
> 		kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device);
> 	...
> 	ret = of_changeset_add_prop_string_array(ocs, np, "compatible",
> 						 compat_strs, PROP_COMPAT_NUM);
> 	...
> }
> 
> If kasprintf() fails, the compat_strs array will contain a null pointer.
> When this array is passed to of_changeset_add_prop_string_array(), it
> unconditionally dereferences the strings to calculate their lengths:
> 
> drivers/of/dynamic.c:of_changeset_add_prop_string_array() {
> 	...
> 	prop.length = 0;
> 	for (i = 0; i < sz; i++)
> 		prop.length += strlen(str_array[i]) + 1;
> 	...
> }
> 
> Would it be better to add a check for allocation failures in
> of_pci_prop_compatible() before passing the array to
> of_changeset_add_prop_string_array() to prevent this?
>