[PATCH v5 0/4] perf/x86/intel/uncore: Fix PCI device reference count leak

[PATCH v5 0/4] perf/x86/intel/uncore: Fix PCI device reference count leak

[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. We add missing pci_dev_put() to decrease the
reference count.


ChangeLog:
v1->v2:
  move pci_dev_put() to the last of function sad_cfg_iio_topology()
  add fixes for snr_uncore_mmio_map(), hswep_has_limit_sbox() and __uncore_imc_init_box()
v2->v3:
  split one patch into several patches, one patch for one fix
v3->v4:
  change 'pci_dev_put(pdev)' to 'pci_dev_put(dev)' in the first patch
v4->v5:
  move the pci_dev_put() to avoid patching it twice

Xiongfeng Wang (4):
  perf/x86/intel/uncore: Fix reference count leak in
    sad_cfg_iio_topology()
  perf/x86/intel/uncore: Fix reference count leak in
    hswep_has_limit_sbox()
  perf/x86/intel/uncore: Fix reference count leak in
    snr_uncore_mmio_map()
  perf/x86/intel/uncore: Fix reference count leak in
    __uncore_imc_init_box()

 arch/x86/events/intel/uncore_snb.c   | 3 +++
 arch/x86/events/intel/uncore_snbep.c | 5 +++++
 2 files changed, 8 insertions(+)

-- 
2.20.1

[PATCH v5 1/4] perf/x86/intel/uncore: Fix reference count leak in sad_cfg_iio_topology()

[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 call pci_dev_put() to decrease the reference count. Since
pci_dev_put() can handle the NULL input parameter, we can just add one
pci_dev_put() right before 'return ret'.

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..76fedc8e12dd 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -4492,6 +4492,8 @@ static int sad_cfg_iio_topology(struct intel_uncore_type *type, u8 *sad_pmon_map
 		type->topology = NULL;
 	}
 
+	pci_dev_put(dev);
+
 	return ret;
 }
 
-- 
2.20.1

[PATCH v5 2/4] perf/x86/intel/uncore: Fix reference count leak in hswep_has_limit_sbox()

[Xiongfeng Wang]

pci_get_device() will increase the reference count for the returned
'dev'. We need to call pci_dev_put() to decrease the reference count.
Since 'dev' is only used in pci_read_config_dword(), let's add
pci_dev_put() right after it.

Fixes: 9d480158ee86 ("perf/x86/intel/uncore: Remove uncore extra PCI dev HSWEP_PCI_PCU_3")
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
---
 arch/x86/events/intel/uncore_snbep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index 76fedc8e12dd..f5d89d06c66a 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -2891,6 +2891,7 @@ static bool hswep_has_limit_sbox(unsigned int device)
 		return false;
 
 	pci_read_config_dword(dev, HSWEP_PCU_CAPID4_OFFET, &capid4);
+	pci_dev_put(dev);
 	if (!hswep_get_chop(capid4))
 		return true;
 
-- 
2.20.1

[PATCH v5 3/4] perf/x86/intel/uncore: Fix reference count leak in snr_uncore_mmio_map()

[Xiongfeng Wang]

pci_get_device() will increase the reference count for the returned
pci_dev, so snr_uncore_get_mc_dev() will return a pci_dev with its
reference count increased. We need to call pci_dev_put() to decrease the
reference count. Let's add the missing pci_dev_put().

Fixes: ee49532b38dd ("perf/x86/intel/uncore: Add IMC uncore support for Snow Ridge")
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 f5d89d06c66a..fcd95e93f479 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -4860,6 +4860,8 @@ static int snr_uncore_mmio_map(struct intel_uncore_box *box,
 
 	addr += box_ctl;
 
+	pci_dev_put(pdev);
+
 	box->io_addr = ioremap(addr, type->mmio_map_size);
 	if (!box->io_addr) {
 		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
-- 
2.20.1

[PATCH v5 4/4] perf/x86/intel/uncore: Fix reference count leak in __uncore_imc_init_box()

[Xiongfeng Wang]

pci_get_device() will increase the reference count for the returned
pci_dev, so tgl_uncore_get_mc_dev() will return a pci_dev with its
reference count increased. We need to call pci_dev_put() to decrease the
reference count before exiting from __uncore_imc_init_box(). Add
pci_dev_put() for both normal and error path.

Fixes: fdb64822443e ("perf/x86: Add Intel Tiger Lake uncore support")
Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
---
 arch/x86/events/intel/uncore_snb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
index 1ef4f7861e2e..1f4869227efb 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -1338,6 +1338,7 @@ static void __uncore_imc_init_box(struct intel_uncore_box *box,
 	/* MCHBAR is disabled */
 	if (!(mch_bar & BIT(0))) {
 		pr_warn("perf uncore: MCHBAR is disabled. Failed to map IMC free-running counters.\n");
+		pci_dev_put(pdev);
 		return;
 	}
 	mch_bar &= ~BIT(0);
@@ -1352,6 +1353,8 @@ static void __uncore_imc_init_box(struct intel_uncore_box *box,
 	box->io_addr = ioremap(addr, type->mmio_map_size);
 	if (!box->io_addr)
 		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
+
+	pci_dev_put(pdev);
 }
 
 static void tgl_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
-- 
2.20.1

Re: [PATCH v5 0/4] perf/x86/intel/uncore: Fix PCI device reference count leak

[Liang, Kan]

On 2022-11-18 1:31 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. We add missing pci_dev_put() to decrease the
> reference count.
> 
> 
> ChangeLog:
> v1->v2:
>   move pci_dev_put() to the last of function sad_cfg_iio_topology()
>   add fixes for snr_uncore_mmio_map(), hswep_has_limit_sbox() and __uncore_imc_init_box()
> v2->v3:
>   split one patch into several patches, one patch for one fix
> v3->v4:
>   change 'pci_dev_put(pdev)' to 'pci_dev_put(dev)' in the first patch
> v4->v5:
>   move the pci_dev_put() to avoid patching it twice

Thanks Xiongfeng. The series looks good to me.

Reviewed-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan
> Xiongfeng Wang (4):
>   perf/x86/intel/uncore: Fix reference count leak in
>     sad_cfg_iio_topology()
>   perf/x86/intel/uncore: Fix reference count leak in
>     hswep_has_limit_sbox()
>   perf/x86/intel/uncore: Fix reference count leak in
>     snr_uncore_mmio_map()
>   perf/x86/intel/uncore: Fix reference count leak in
>     __uncore_imc_init_box()
> 
>  arch/x86/events/intel/uncore_snb.c   | 3 +++
>  arch/x86/events/intel/uncore_snbep.c | 5 +++++
>  2 files changed, 8 insertions(+)
>