[PATCH] perf/x86/intel/uncore: decrease reference count in error path

[PATCH] perf/x86/intel/uncore: decrease reference count in error path

[Xiongfeng Wang]

pci_get_device() will increase the reference count for the returned
pci_dev, and also decrease the reference count for the input parameter
*from* if it is not NULL.

If we break the loop in sad_cfg_iio_topology() with 'dev' not NULL. We
need to use pci_dev_put() to decrease the reference count. Let's add it.

Fixes: c1777be3646b ("perf/x86/intel/uncore: Enable I/O stacks to IIO PMON mapping on SNR")
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
---
 arch/x86/events/intel/uncore_snbep.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index ed869443efb2..83fe3d80c9a7 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -4469,6 +4469,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
 	while ((dev = pci_get_device(PCI_VENDOR_ID_INTEL, SNR_ICX_MESH2IIO_MMAP_DID, dev))) {
 		ret = pci_read_config_dword(dev, SNR_ICX_SAD_CONTROL_CFG, &sad_cfg);
 		if (ret) {
+			pci_dev_put(dev);
 			ret = pcibios_err_to_errno(ret);
 			break;
 		}
@@ -4476,6 +4477,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
 		die = uncore_pcibus_to_dieid(dev->bus);
 		stack_id = SAD_CONTROL_STACK_ID(sad_cfg);
 		if (die < 0 || stack_id >= type->num_boxes) {
+			pci_dev_put(dev);
 			ret = -EPERM;
 			break;
 		}
-- 
2.20.1

Re: [PATCH] perf/x86/intel/uncore: decrease reference count in error path

[Liang, Kan]

On 2022-11-15 6:25 a.m., Xiongfeng Wang wrote:
> pci_get_device() will increase the reference count for the returned
> pci_dev, and also decrease the reference count for the input parameter
> *from* if it is not NULL.
> 
> If we break the loop in sad_cfg_iio_topology() with 'dev' not NULL. We
> need to use pci_dev_put() to decrease the reference count. Let's add it.
> 
> Fixes: c1777be3646b ("perf/x86/intel/uncore: Enable I/O stacks to IIO PMON mapping on SNR")
> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
> ---
>  arch/x86/events/intel/uncore_snbep.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> index ed869443efb2..83fe3d80c9a7 100644
> --- a/arch/x86/events/intel/uncore_snbep.c
> +++ b/arch/x86/events/intel/uncore_snbep.c
> @@ -4469,6 +4469,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
>  	while ((dev = pci_get_device(PCI_VENDOR_ID_INTEL, SNR_ICX_MESH2IIO_MMAP_DID, dev))) {
>  		ret = pci_read_config_dword(dev, SNR_ICX_SAD_CONTROL_CFG, &sad_cfg);
>  		if (ret) {
> +			pci_dev_put(dev);
>  			ret = pcibios_err_to_errno(ret);
>  			break;
>  		}
> @@ -4476,6 +4477,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
>  		die = uncore_pcibus_to_dieid(dev->bus);
>  		stack_id = SAD_CONTROL_STACK_ID(sad_cfg);
>  		if (die < 0 || stack_id >= type->num_boxes) {
> +			pci_dev_put(dev);
>  			ret = -EPERM;
>  			break;
>  		}

I think the pci_dev_put() itself checks the NULL. So we may just need to
add one pci_dev_put() right before return ret;.

Thanks,
Kan

Re: [PATCH] perf/x86/intel/uncore: decrease reference count in error path

[Alexander Antonov]

On 11/15/2022 12:25 PM, Xiongfeng Wang wrote:
> pci_get_device() will increase the reference count for the returned
> pci_dev, and also decrease the reference count for the input parameter
> *from* if it is not NULL.
>
> If we break the loop in sad_cfg_iio_topology() with 'dev' not NULL. We
> need to use pci_dev_put() to decrease the reference count. Let's add it.
>
> Fixes: c1777be3646b ("perf/x86/intel/uncore: Enable I/O stacks to IIO PMON mapping on SNR")
> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
> ---
>   arch/x86/events/intel/uncore_snbep.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> index ed869443efb2..83fe3d80c9a7 100644
> --- a/arch/x86/events/intel/uncore_snbep.c
> +++ b/arch/x86/events/intel/uncore_snbep.c
> @@ -4469,6 +4469,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
>   	while ((dev = pci_get_device(PCI_VENDOR_ID_INTEL, SNR_ICX_MESH2IIO_MMAP_DID, dev))) {
>   		ret = pci_read_config_dword(dev, SNR_ICX_SAD_CONTROL_CFG, &sad_cfg);
>   		if (ret) {
> +			pci_dev_put(dev);
>   			ret = pcibios_err_to_errno(ret);
>   			break;
>   		}
> @@ -4476,6 +4477,7 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
>   		die = uncore_pcibus_to_dieid(dev->bus);
>   		stack_id = SAD_CONTROL_STACK_ID(sad_cfg);
>   		if (die < 0 || stack_id >= type->num_boxes) {
> +			pci_dev_put(dev);
>   			ret = -EPERM;
>   			break;
>   		}
Hi Xiongfeng,

Could you please update snr_uncore_mmio_map() and hswep_has_limit_sbox() 
as well?
pci_dev_put() is missed there also

Thanks,
Alexander