* [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).