netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] octeontx2: Fix several bugs in exception paths
@ 2022-11-24  1:56 Ziyang Xuan
  2022-11-24  1:56 ` [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe() Ziyang Xuan
  2022-11-24  1:56 ` [PATCH net 2/2] octeontx2-vf: Fix possible memory leak in otx2vf_probe() Ziyang Xuan
  0 siblings, 2 replies; 9+ messages in thread
From: Ziyang Xuan @ 2022-11-24  1:56 UTC (permalink / raw)
  To: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni, netdev
  Cc: naveenm, rsaladi2, linux-kernel

Find several obvious bugs during code review in exception paths. Provide
this patchset to fix them. Not tested, just compiled.

Ziyang Xuan (2):
  octeontx2-pf: Fix possible memory leak in otx2_probe()
  octeontx2-vf: Fix possible memory leak in otx2vf_probe()

 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c |  5 ++++-
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 12 +++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-11-24  1:56 [PATCH net 0/2] octeontx2: Fix several bugs in exception paths Ziyang Xuan
@ 2022-11-24  1:56 ` Ziyang Xuan
  2022-11-25 13:46   ` Maciej Fijalkowski
  2022-11-24  1:56 ` [PATCH net 2/2] octeontx2-vf: Fix possible memory leak in otx2vf_probe() Ziyang Xuan
  1 sibling, 1 reply; 9+ messages in thread
From: Ziyang Xuan @ 2022-11-24  1:56 UTC (permalink / raw)
  To: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni, netdev
  Cc: naveenm, rsaladi2, linux-kernel

In otx2_probe(), there are several possible memory leak bugs
in exception paths as follows:
1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
2. Do not shutdown tc when excute otx2_register_dl() failed.
3. Do not unregister devlink when initialize SR-IOV failed.

Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 303930499a4c..8d7f2c3b0cfd 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	err = otx2_register_dl(pf);
 	if (err)
-		goto err_mcam_flow_del;
+		goto err_register_dl;
 
 	/* Initialize SR-IOV resources */
 	err = otx2_sriov_vfcfg_init(pf);
@@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return 0;
 
 err_pf_sriov_init:
+	otx2_unregister_dl(pf);
+err_register_dl:
 	otx2_shutdown_tc(pf);
 err_mcam_flow_del:
+	destroy_workqueue(pf->otx2_wq);
 	otx2_mcam_flow_del(pf);
 err_unreg_netdev:
 	unregister_netdev(netdev);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 2/2] octeontx2-vf: Fix possible memory leak in otx2vf_probe()
  2022-11-24  1:56 [PATCH net 0/2] octeontx2: Fix several bugs in exception paths Ziyang Xuan
  2022-11-24  1:56 ` [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe() Ziyang Xuan
@ 2022-11-24  1:56 ` Ziyang Xuan
  1 sibling, 0 replies; 9+ messages in thread
From: Ziyang Xuan @ 2022-11-24  1:56 UTC (permalink / raw)
  To: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni, netdev
  Cc: naveenm, rsaladi2, linux-kernel

In otx2vf_probe(), there are several possible memory leak bugs
in exception paths as follows:
1. Do not release vf->otx2_wq when excute otx2vf_mcam_flow_init() and
   otx2_init_tc() failed.
2. Do not unregister devlink when excute otx2_dcbnl_set_ops() failed.

Fixes: 4b0385bc8e6a ("octeontx2-pf: Add TC feature for VFs")
Fixes: 8e67558177f8 ("octeontx2-pf: PFC config support with DCBx")
Fixes: 3cffaed2136c ("octeontx2-pf: Ntuple filters support for VF netdev")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index 86653bb8e403..f1b47fecd379 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -684,11 +684,11 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	err = otx2vf_mcam_flow_init(vf);
 	if (err)
-		goto err_unreg_netdev;
+		goto err_destroy_wq;
 
 	err = otx2_init_tc(vf);
 	if (err)
-		goto err_unreg_netdev;
+		goto err_destroy_wq;
 
 	err = otx2_register_dl(vf);
 	if (err)
@@ -697,13 +697,19 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 #ifdef CONFIG_DCB
 	err = otx2_dcbnl_set_ops(netdev);
 	if (err)
-		goto err_shutdown_tc;
+		goto err_unreg_dl;
 #endif
 
 	return 0;
 
+#ifdef CONFIG_DCB
+err_unreg_dl:
+	otx2_unregister_dl(vf);
+#endif
 err_shutdown_tc:
 	otx2_shutdown_tc(vf);
+err_destroy_wq:
+	destroy_workqueue(vf->otx2_wq);
 err_unreg_netdev:
 	unregister_netdev(netdev);
 err_ptp_destroy:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-11-24  1:56 ` [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe() Ziyang Xuan
@ 2022-11-25 13:46   ` Maciej Fijalkowski
  2022-11-26  1:04     ` Ziyang Xuan (William)
  0 siblings, 1 reply; 9+ messages in thread
From: Maciej Fijalkowski @ 2022-11-25 13:46 UTC (permalink / raw)
  To: Ziyang Xuan
  Cc: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni,
	netdev, naveenm, rsaladi2, linux-kernel

On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
> In otx2_probe(), there are several possible memory leak bugs
> in exception paths as follows:
> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
> 2. Do not shutdown tc when excute otx2_register_dl() failed.
> 3. Do not unregister devlink when initialize SR-IOV failed.
> 
> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index 303930499a4c..8d7f2c3b0cfd 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	err = otx2_register_dl(pf);
>  	if (err)
> -		goto err_mcam_flow_del;
> +		goto err_register_dl;
>  
>  	/* Initialize SR-IOV resources */
>  	err = otx2_sriov_vfcfg_init(pf);
> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	return 0;

If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
otx2_sriov_vfcfg_cleanup() ?

>  
>  err_pf_sriov_init:
> +	otx2_unregister_dl(pf);
> +err_register_dl:
>  	otx2_shutdown_tc(pf);
>  err_mcam_flow_del:
> +	destroy_workqueue(pf->otx2_wq);
>  	otx2_mcam_flow_del(pf);
>  err_unreg_netdev:
>  	unregister_netdev(netdev);
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-11-25 13:46   ` Maciej Fijalkowski
@ 2022-11-26  1:04     ` Ziyang Xuan (William)
  2022-12-02  1:44       ` Ziyang Xuan (William)
  0 siblings, 1 reply; 9+ messages in thread
From: Ziyang Xuan (William) @ 2022-11-26  1:04 UTC (permalink / raw)
  To: Maciej Fijalkowski, sunil.kovvuri, sgoutham
  Cc: gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni, netdev,
	naveenm, rsaladi2, linux-kernel

> On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
>> In otx2_probe(), there are several possible memory leak bugs
>> in exception paths as follows:
>> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
>> 2. Do not shutdown tc when excute otx2_register_dl() failed.
>> 3. Do not unregister devlink when initialize SR-IOV failed.
>>
>> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
>> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>> ---
>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> index 303930499a4c..8d7f2c3b0cfd 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>  
>>  	err = otx2_register_dl(pf);
>>  	if (err)
>> -		goto err_mcam_flow_del;
>> +		goto err_register_dl;
>>  
>>  	/* Initialize SR-IOV resources */
>>  	err = otx2_sriov_vfcfg_init(pf);
>> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>  	return 0;
> 
> If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
> otx2_sriov_vfcfg_cleanup() ?

I think it does not need. This is the probe process. PF and VF are all not ready to work,
so pf->vf_configs[i].link_event_work does not scheduled. And pf->vf_configs memory resource will
be freed by devm subsystem if probe failed. There are not memory leak and other problems.

@Sunil Goutham, Please help to confirm.

Thanks.

> 
>>  
>>  err_pf_sriov_init:
>> +	otx2_unregister_dl(pf);
>> +err_register_dl:
>>  	otx2_shutdown_tc(pf);
>>  err_mcam_flow_del:
>> +	destroy_workqueue(pf->otx2_wq);
>>  	otx2_mcam_flow_del(pf);
>>  err_unreg_netdev:
>>  	unregister_netdev(netdev);
>> -- 
>> 2.25.1
>>
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-11-26  1:04     ` Ziyang Xuan (William)
@ 2022-12-02  1:44       ` Ziyang Xuan (William)
  2022-12-02  6:38         ` [EXT] " Geethasowjanya Akula
  0 siblings, 1 reply; 9+ messages in thread
From: Ziyang Xuan (William) @ 2022-12-02  1:44 UTC (permalink / raw)
  To: Maciej Fijalkowski, sunil.kovvuri, sgoutham
  Cc: gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni, netdev,
	naveenm, rsaladi2, linux-kernel

>> On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
>>> In otx2_probe(), there are several possible memory leak bugs
>>> in exception paths as follows:
>>> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
>>> 2. Do not shutdown tc when excute otx2_register_dl() failed.
>>> 3. Do not unregister devlink when initialize SR-IOV failed.
>>>
>>> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
>>> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>>> ---
>>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>> index 303930499a4c..8d7f2c3b0cfd 100644
>>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>  
>>>  	err = otx2_register_dl(pf);
>>>  	if (err)
>>> -		goto err_mcam_flow_del;
>>> +		goto err_register_dl;
>>>  
>>>  	/* Initialize SR-IOV resources */
>>>  	err = otx2_sriov_vfcfg_init(pf);
>>> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>  	return 0;
>>
>> If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
>> otx2_sriov_vfcfg_cleanup() ?
> 
> I think it does not need. This is the probe process. PF and VF are all not ready to work,
> so pf->vf_configs[i].link_event_work does not scheduled. And pf->vf_configs memory resource will
> be freed by devm subsystem if probe failed. There are not memory leak and other problems.
> 
Hello Sunil Goutham, Maciej Fijalkowski,

What do you think about my analysis? Look forward to your reply.

Thank you!

> @Sunil Goutham, Please help to confirm.
> 
> Thanks.
> 
>>
>>>  
>>>  err_pf_sriov_init:
>>> +	otx2_unregister_dl(pf);
>>> +err_register_dl:
>>>  	otx2_shutdown_tc(pf);
>>>  err_mcam_flow_del:
>>> +	destroy_workqueue(pf->otx2_wq);
>>>  	otx2_mcam_flow_del(pf);
>>>  err_unreg_netdev:
>>>  	unregister_netdev(netdev);
>>> -- 
>>> 2.25.1
>>>
>> .
>>
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-12-02  1:44       ` Ziyang Xuan (William)
@ 2022-12-02  6:38         ` Geethasowjanya Akula
  2022-12-02  8:34           ` Ziyang Xuan (William)
  0 siblings, 1 reply; 9+ messages in thread
From: Geethasowjanya Akula @ 2022-12-02  6:38 UTC (permalink / raw)
  To: Ziyang Xuan (William),
	Maciej Fijalkowski, sunil.kovvuri, Sunil Kovvuri Goutham
  Cc: Subbaraya Sundeep Bhatta, Hariprasad Kelam, davem, edumazet,
	kuba, pabeni, netdev, Naveen Mamindlapalli, Rakesh Babu Saladi,
	linux-kernel


________________________________________
From: Ziyang Xuan (William) <william.xuanziyang@huawei.com>
Sent: Friday, December 2, 2022 7:14 AM
To: Maciej Fijalkowski; sunil.kovvuri@gmail.com; Sunil Kovvuri Goutham
Cc: Geethasowjanya Akula; Subbaraya Sundeep Bhatta; Hariprasad Kelam; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Naveen Mamindlapalli; Rakesh Babu Saladi; linux-kernel@vger.kernel.org
Subject: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()

External Email

----------------------------------------------------------------------
>>> On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
>>>> In otx2_probe(), there are several possible memory leak bugs
>>>> in exception paths as follows:
>>>> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
>>>> 2. Do not shutdown tc when excute otx2_register_dl() failed.
>>>> 3. Do not unregister devlink when initialize SR-IOV failed.
>>>>
>>>> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
>>>> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
>>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>>>> ---
>>>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>> index 303930499a4c..8d7f2c3b0cfd 100644
>>>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>
>>>>     err = otx2_register_dl(pf);
>>>>     if (err)
>>>> -           goto err_mcam_flow_del;
>>>> +           goto err_register_dl;
>>>>
>>>>     /* Initialize SR-IOV resources */
>>>>     err = otx2_sriov_vfcfg_init(pf);
>>>> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>     return 0;
>>>
>>> If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
>>> otx2_sriov_vfcfg_cleanup() ?
>>
>> I think it does not need. This is the probe process. PF and VF are all not ready to work,
>> so pf->vf_configs[i].link_event_work does not scheduled. And pf->vf_configs memory resource will
>> be freed by devm subsystem if probe failed. There are not memory leak and other problems.
>>
>Hello Sunil Goutham, Maciej Fijalkowski,

>What do you think about my analysis? Look forward to your >reply.
otx2_sriov_vfcfg_cleanup() is not required. Since PF probe is failed, link event won't get triggered.

Thanks,
Geetha.
>Thank you!

>> @Sunil Goutham, Please help to confirm.
>>
>> Thanks.
>>
>>>
>>>>
>>>>  err_pf_sriov_init:
>>>> +   otx2_unregister_dl(pf);
>>>> +err_register_dl:
>>>>     otx2_shutdown_tc(pf);
>>>>  err_mcam_flow_del:
>>>> +   destroy_workqueue(pf->otx2_wq);
>>>>     otx2_mcam_flow_del(pf);
>>>>  err_unreg_netdev:
>>>>     unregister_netdev(netdev);
>>>> --
>>>> 2.25.1
>>>>
>>> .
>>>
>> .
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-12-02  6:38         ` [EXT] " Geethasowjanya Akula
@ 2022-12-02  8:34           ` Ziyang Xuan (William)
  2022-12-02  9:50             ` Geethasowjanya Akula
  0 siblings, 1 reply; 9+ messages in thread
From: Ziyang Xuan (William) @ 2022-12-02  8:34 UTC (permalink / raw)
  To: Geethasowjanya Akula, Maciej Fijalkowski, sunil.kovvuri,
	Sunil Kovvuri Goutham
  Cc: Subbaraya Sundeep Bhatta, Hariprasad Kelam, davem, edumazet,
	kuba, pabeni, netdev, Naveen Mamindlapalli, Rakesh Babu Saladi,
	linux-kernel

> 
> ________________________________________
> From: Ziyang Xuan (William) <william.xuanziyang@huawei.com>
> Sent: Friday, December 2, 2022 7:14 AM
> To: Maciej Fijalkowski; sunil.kovvuri@gmail.com; Sunil Kovvuri Goutham
> Cc: Geethasowjanya Akula; Subbaraya Sundeep Bhatta; Hariprasad Kelam; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Naveen Mamindlapalli; Rakesh Babu Saladi; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
> 
> External Email
> 
> ----------------------------------------------------------------------
>>>> On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
>>>>> In otx2_probe(), there are several possible memory leak bugs
>>>>> in exception paths as follows:
>>>>> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
>>>>> 2. Do not shutdown tc when excute otx2_register_dl() failed.
>>>>> 3. Do not unregister devlink when initialize SR-IOV failed.
>>>>>
>>>>> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
>>>>> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
>>>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>>>>> ---
>>>>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>> index 303930499a4c..8d7f2c3b0cfd 100644
>>>>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>>
>>>>>     err = otx2_register_dl(pf);
>>>>>     if (err)
>>>>> -           goto err_mcam_flow_del;
>>>>> +           goto err_register_dl;
>>>>>
>>>>>     /* Initialize SR-IOV resources */
>>>>>     err = otx2_sriov_vfcfg_init(pf);
>>>>> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>>     return 0;
>>>>
>>>> If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
>>>> otx2_sriov_vfcfg_cleanup() ?
>>>
>>> I think it does not need. This is the probe process. PF and VF are all not ready to work,
>>> so pf->vf_configs[i].link_event_work does not scheduled. And pf->vf_configs memory resource will
>>> be freed by devm subsystem if probe failed. There are not memory leak and other problems.
>>>
>> Hello Sunil Goutham, Maciej Fijalkowski,
> 
>> What do you think about my analysis? Look forward to your >reply.
> otx2_sriov_vfcfg_cleanup() is not required. Since PF probe is failed, link event won't get triggered.
> 
Hello Geetha,

If there is not any other question, can you add "Reviewed-by" for my patchset?

Thank you!

> Thanks,
> Geetha.
>> Thank you!
> 
>>> @Sunil Goutham, Please help to confirm.
>>>
>>> Thanks.
>>>
>>>>
>>>>>
>>>>>  err_pf_sriov_init:
>>>>> +   otx2_unregister_dl(pf);
>>>>> +err_register_dl:
>>>>>     otx2_shutdown_tc(pf);
>>>>>  err_mcam_flow_del:
>>>>> +   destroy_workqueue(pf->otx2_wq);
>>>>>     otx2_mcam_flow_del(pf);
>>>>>  err_unreg_netdev:
>>>>>     unregister_netdev(netdev);
>>>>> --
>>>>> 2.25.1
>>>>>
>>>> .
>>>>
>>> .
>>>
> .
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
  2022-12-02  8:34           ` Ziyang Xuan (William)
@ 2022-12-02  9:50             ` Geethasowjanya Akula
  0 siblings, 0 replies; 9+ messages in thread
From: Geethasowjanya Akula @ 2022-12-02  9:50 UTC (permalink / raw)
  To: Ziyang Xuan (William),
	Maciej Fijalkowski, sunil.kovvuri, Sunil Kovvuri Goutham
  Cc: Subbaraya Sundeep Bhatta, Hariprasad Kelam, davem, edumazet,
	kuba, pabeni, netdev, Naveen Mamindlapalli, Rakesh Babu Saladi,
	linux-kernel

________________________________________
From: Ziyang Xuan (William) <william.xuanziyang@huawei.com>
Sent: Friday, December 2, 2022 2:04 PM
To: Geethasowjanya Akula; Maciej Fijalkowski; sunil.kovvuri@gmail.com; Sunil Kovvuri Goutham
Cc: Subbaraya Sundeep Bhatta; Hariprasad Kelam; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Naveen Mamindlapalli; Rakesh Babu Saladi; linux-kernel@vger.kernel.org
Subject: Re: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()

>
> ________________________________________
> From: Ziyang Xuan (William) <william.xuanziyang@huawei.com>
> Sent: Friday, December 2, 2022 7:14 AM
> To: Maciej Fijalkowski; sunil.kovvuri@gmail.com; Sunil Kovvuri Goutham
> Cc: Geethasowjanya Akula; Subbaraya Sundeep Bhatta; Hariprasad Kelam; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; Naveen Mamindlapalli; Rakesh Babu Saladi; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe()
>
> External Email
>
> ----------------------------------------------------------------------
>>>>> On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
>>>>>> In otx2_probe(), there are several possible memory leak bugs
>>>>>> in exception paths as follows:
>>>>>> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
>>>>>> 2. Do not shutdown tc when excute otx2_register_dl() failed.
>>>>>> 3. Do not unregister devlink when initialize SR-IOV failed.
>>>>>>
>>>>>> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
>>>>>> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
>>>>>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
>>>>>> ---
>>>>>>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>>> index 303930499a4c..8d7f2c3b0cfd 100644
>>>>>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>>>>> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>>>
>>>>>>     err = otx2_register_dl(pf);
>>>>>>     if (err)
>>>>>> -           goto err_mcam_flow_del;
>>>>>> +           goto err_register_dl;
>>>>>>
>>>>>>     /* Initialize SR-IOV resources */
>>>>>>     err = otx2_sriov_vfcfg_init(pf);
>>>>>> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>>>>     return 0;
>>>>>
>>>>> If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
>>>>> otx2_sriov_vfcfg_cleanup() ?
>>>>
>>>> I think it does not need. This is the probe process. PF and VF are all not ready to work,
>>>> so pf->vf_configs[i].link_event_work does not scheduled. And pf->vf_configs memory resource will
>>>> be freed by devm subsystem if probe failed. There are not memory leak and other problems.
>>>>
>>> Hello Sunil Goutham, Maciej Fijalkowski,
>>
>>> What do you think about my analysis? Look forward to your >>reply.
>> otx2_sriov_vfcfg_cleanup() is not required. Since PF probe is failed, link event won't get triggered.
>>
>Hello Geetha,

>If there is not any other question, can you add "Reviewed-by" >for my patchset?

>Thank you!

>> Thanks,
>> Geetha.
>> Thank you!
>>
>>>> @Sunil Goutham, Please help to confirm.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>>>
>>>>>  err_pf_sriov_init:
>>>>> +   otx2_unregister_dl(pf);
>>>>> +err_register_dl:
>>>>>     otx2_shutdown_tc(pf);
>>>>>  err_mcam_flow_del:
>>>>> +   destroy_workqueue(pf->otx2_wq);
>>>>>     otx2_mcam_flow_del(pf);
>>>>>  err_unreg_netdev:
>>>>>     unregister_netdev(netdev);
>>>>> --
>>>>> 2.25.1
>>>>>
>>>> .
>>>>
>>> .
>>>
> .
>

Reviewed-by: Geethasowjanya<gakula@marvell.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-12-02  9:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24  1:56 [PATCH net 0/2] octeontx2: Fix several bugs in exception paths Ziyang Xuan
2022-11-24  1:56 ` [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in otx2_probe() Ziyang Xuan
2022-11-25 13:46   ` Maciej Fijalkowski
2022-11-26  1:04     ` Ziyang Xuan (William)
2022-12-02  1:44       ` Ziyang Xuan (William)
2022-12-02  6:38         ` [EXT] " Geethasowjanya Akula
2022-12-02  8:34           ` Ziyang Xuan (William)
2022-12-02  9:50             ` Geethasowjanya Akula
2022-11-24  1:56 ` [PATCH net 2/2] octeontx2-vf: Fix possible memory leak in otx2vf_probe() Ziyang Xuan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).