[PATCH] ocxl: fix pci device refcount leak when calling get_function_0()

[Yang Yingliang]

Hi,

On 2022/11/21 14:28, Christophe Leroy wrote:
>
> Le 19/11/2022 à 15:44, Yang Yingliang a écrit :
>> As comment of pci_get_domain_bus_and_slot() says, it returns
>> a pci device with refcount increment, so when finish using it,
>> pci_dev_put() needs be called.
>>
>> In get_dvsec_vendor0(), in normal path, the returned pci device
>> is passed to dev0, so after using dev0 in the callers, it need
>> be put, in error path, pci_dev_put() also needs be called.
>>
>> pci_get_domain_bus_and_slot() is called when PCI_FUNC() returns
>> non-zero, check this before put.
>>
>> Fixes: 87db7579ebd5 ("ocxl: control via sysfs whether the FPGA is reloaded on a link reset")
>> Signed-off-by: Yang Yingliang <yangyingliang at huawei.com>
>> ---
>>    drivers/misc/ocxl/config.c | 11 ++++++++++-
>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index e401a51596b9..4da5a2b8514c 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -196,16 +196,21 @@ static int read_dvsec_vendor(struct pci_dev *dev)
>>    static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0,
>>    			     int *out_pos)
>>    {
>> +	bool need_put;
>>    	int pos;
>>    
>>    	if (PCI_FUNC(dev->devfn) != 0) {
>>    		dev = get_function_0(dev);
>>    		if (!dev)
>>    			return -1;
>> +		need_put = true;
> Why introduce that 'need_put' boolean ? Why not just use
> PCI_FUNC(dev->devfn) != 0 as you do in other places ?
The 'dev' is reassigned by get_function_0(), it is not the origin one.

Thanks,
Yang
>
>>    	}
>>    	pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID);
>> -	if (!pos)
>> +	if (!pos) {
>> +		if (need_put)
>> +			pci_dev_put(dev);
>>    		return -1;
>> +	}
>>    	*dev0 = dev;
>>    	*out_pos = pos;
>>    	return 0;
>> @@ -222,6 +227,8 @@ int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val)
>>    
>>    	pci_read_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
>>    			      &reset_reload);
>> +	if (PCI_FUNC(dev->devfn) != 0)
> The != 0 is useless, just do:
>
> 	if (PCI_FUNC(dev->devfn))
>
>> +		pci_dev_put(dev0);
>>    	*val = !!(reset_reload & BIT(0));
>>    	return 0;
>>    }
>> @@ -243,6 +250,8 @@ int ocxl_config_set_reset_reload(struct pci_dev *dev, int val)
>>    		reset_reload &= ~BIT(0);
>>    	pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD,
>>    			       reset_reload);
>> +	if (PCI_FUNC(dev->devfn) != 0)
> Same
>
>> +		pci_dev_put(dev0);
>>    	return 0;
>>    }
>>