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