diff mbox series

[v1] net/iavf: fix resource leak issue

Message ID 20220510005425.1648591-1-wenjun1.wu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v1] net/iavf: fix resource leak issue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Wenjun Wu May 10, 2022, 12:54 a.m. UTC 
  This patch fixes resource leak issue reported by coverity.

Coverity issue: 378017
Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")

Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
---
 drivers/net/iavf/iavf_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Qi Zhang May 11, 2022, 11:46 a.m. UTC | #1 
> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Tuesday, May 10, 2022 8:54 AM
> To: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: [PATCH v1] net/iavf: fix resource leak issue
> 
> This patch fixes resource leak issue reported by coverity.
> 
> Coverity issue: 378017
> Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> 
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
Thomas Monjalon May 11, 2022, 2:52 p.m. UTC | #2 
11/05/2022 13:46, Zhang, Qi Z:
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > 
> > This patch fixes resource leak issue reported by coverity.
> > 
> > Coverity issue: 378017
> > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > 
> > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.

In the title, the word "issue" is useless.
It would be more interesting to precise where is the leak.
Qi Zhang May 11, 2022, 3:31 p.m. UTC | #3 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, May 11, 2022 10:52 PM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 11/05/2022 13:46, Zhang, Qi Z:
> > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > >
> > > This patch fixes resource leak issue reported by coverity.
> > >
> > > Coverity issue: 378017
> > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > >
> > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > Applied to dpdk-next-net-intel.
> 
> In the title, the word "issue" is useless.
> It would be more interesting to precise where is the leak.
> 

Added below into commit log in dpdk-next-net-intel.

kvlist should be freed before return an error due to invalid quanta size.
Thomas Monjalon May 11, 2022, 9:17 p.m. UTC | #4 
11/05/2022 17:31, Zhang, Qi Z:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/05/2022 13:46, Zhang, Qi Z:
> > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > >
> > > > This patch fixes resource leak issue reported by coverity.
> > > >
> > > > Coverity issue: 378017
> > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > >
> > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > >
> > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > >
> > > Applied to dpdk-next-net-intel.
> > 
> > In the title, the word "issue" is useless.
> > It would be more interesting to precise where is the leak.
> > 
> 
> Added below into commit log in dpdk-next-net-intel.
> 
> kvlist should be freed before return an error due to invalid quanta size. 

It does not explain what is the scenario. What is the impacted feature?
Qi Zhang May 12, 2022, 12:20 a.m. UTC | #5 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, May 12, 2022 5:17 AM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 11/05/2022 17:31, Zhang, Qi Z:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > >
> > > > > This patch fixes resource leak issue reported by coverity.
> > > > >
> > > > > Coverity issue: 378017
> > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > > >
> > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > >
> > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > >
> > > > Applied to dpdk-next-net-intel.
> > >
> > > In the title, the word "issue" is useless.
> > > It would be more interesting to precise where is the leak.
> > >
> >
> > Added below into commit log in dpdk-next-net-intel.
> >
> > kvlist should be freed before return an error due to invalid quanta size.
> 
> It does not explain what is the scenario. What is the impacted feature?

I think the scenario has been explained as

"kvlist should be freed before return an error due to invalid quanta size"

the impact has been mentioned as "resource leak" I guess this is how Coverity describe the issue

i agree "memory leak" would be better.

>
Thomas Monjalon May 12, 2022, 7:06 a.m. UTC | #6 
12/05/2022 02:20, Zhang, Qi Z:
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, May 12, 2022 5:17 AM
> > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > 
> > 11/05/2022 17:31, Zhang, Qi Z:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > >
> > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > >
> > > > > > Coverity issue: 378017
> > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size configuration")
> > > > > >
> > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > >
> > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > >
> > > > > Applied to dpdk-next-net-intel.
> > > >
> > > > In the title, the word "issue" is useless.
> > > > It would be more interesting to precise where is the leak.
> > > >
> > >
> > > Added below into commit log in dpdk-next-net-intel.
> > >
> > > kvlist should be freed before return an error due to invalid quanta size.
> > 
> > It does not explain what is the scenario. What is the impacted feature?
> 
> I think the scenario has been explained as
> 
> "kvlist should be freed before return an error due to invalid quanta size"
> the impact has been mentioned as "resource leak" I guess this is how Coverity describe the issue

No, it does not say how to reproduce (the scenario).
We have no idea where it is in the code.
Qi Zhang May 12, 2022, 7:21 a.m. UTC | #7 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, May 12, 2022 3:07 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 12/05/2022 02:20, Zhang, Qi Z:
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Thursday, May 12, 2022 5:17 AM
> > > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>
> > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>
> > > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > >
> > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > >
> > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > >
> > > > > > > Coverity issue: 378017
> > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > configuration")
> > > > > > >
> > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > >
> > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > >
> > > > > > Applied to dpdk-next-net-intel.
> > > > >
> > > > > In the title, the word "issue" is useless.
> > > > > It would be more interesting to precise where is the leak.
> > > > >
> > > >
> > > > Added below into commit log in dpdk-next-net-intel.
> > > >
> > > > kvlist should be freed before return an error due to invalid quanta size.
> > >
> > > It does not explain what is the scenario. What is the impacted feature?
> >
> > I think the scenario has been explained as
> >
> > "kvlist should be freed before return an error due to invalid quanta size"
> > the impact has been mentioned as "resource leak" I guess this is how
> > Coverity describe the issue
> 
> No, it does not say how to reproduce (the scenario).
> We have no idea where it is in the code.

set an invalid quanta size from devargs will reproduce the issue
> 
>
Qi Zhang May 17, 2022, 12:59 a.m. UTC | #8 
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Thursday, May 12, 2022 3:21 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: RE: [PATCH v1] net/iavf: fix resource leak issue
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, May 12, 2022 3:07 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> > <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> >
> > 12/05/2022 02:20, Zhang, Qi Z:
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Thursday, May 12, 2022 5:17 AM
> > > > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Zhang, Qi Z
> > > > <qi.z.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> > > > Beilei <beilei.xing@intel.com>
> > > > Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> > > >
> > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > >
> > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > >
> > > > > > > > Coverity issue: 378017
> > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > configuration")
> > > > > > > >
> > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > >
> > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > >
> > > > > > > Applied to dpdk-next-net-intel.
> > > > > >
> > > > > > In the title, the word "issue" is useless.
> > > > > > It would be more interesting to precise where is the leak.
> > > > > >
> > > > >
> > > > > Added below into commit log in dpdk-next-net-intel.
> > > > >
> > > > > kvlist should be freed before return an error due to invalid quanta size.
> > > >
> > > > It does not explain what is the scenario. What is the impacted feature?
> > >
> > > I think the scenario has been explained as
> > >
> > > "kvlist should be freed before return an error due to invalid quanta size"
> > > the impact has been mentioned as "resource leak" I guess this is how
> > > Coverity describe the issue
> >
> > No, it does not say how to reproduce (the scenario).
> > We have no idea where it is in the code.
> 
> set an invalid quanta size from devargs will reproduce the issue
> >
> >
Refine the Title / commit log in dpdk-next-net-intel as below to answer Thomas's questions

net/iavf: fix memory leak issue

Set an invalid quanta size from devargs will cause memory leak and this
is reported by coverity.

The patch fix the issue by correcting the error handle.
Thomas Monjalon May 17, 2022, 6:52 a.m. UTC | #9 
17/05/2022 02:59, Zhang, Qi Z:
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 12/05/2022 02:20, Zhang, Qi Z:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > > >
> > > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > > >
> > > > > > > > > Coverity issue: 378017
> > > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > > configuration")
> > > > > > > > >
> > > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > > >
> > > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > > >
> > > > > > > > Applied to dpdk-next-net-intel.
> > > > > > >
> > > > > > > In the title, the word "issue" is useless.
> > > > > > > It would be more interesting to precise where is the leak.
> > > > > > >
> > > > > >
> > > > > > Added below into commit log in dpdk-next-net-intel.
> > > > > >
> > > > > > kvlist should be freed before return an error due to invalid quanta size.
> > > > >
> > > > > It does not explain what is the scenario. What is the impacted feature?
> > > >
> > > > I think the scenario has been explained as
> > > >
> > > > "kvlist should be freed before return an error due to invalid quanta size"
> > > > the impact has been mentioned as "resource leak" I guess this is how
> > > > Coverity describe the issue
> > >
> > > No, it does not say how to reproduce (the scenario).
> > > We have no idea where it is in the code.
> > 
> > set an invalid quanta size from devargs will reproduce the issue
> > >
> > >
> Refine the Title / commit log in dpdk-next-net-intel as below to answer Thomas's questions
> 
> net/iavf: fix memory leak issue

A memory leak is an issue for sure, so no need to add "issue".

> Set an invalid quanta size from devargs will cause memory leak and this

Set -> Setting

> is reported by coverity.
> 
> The patch fix the issue by correcting the error handle.

handle -> handling

Thanks
Qi Zhang May 18, 2022, 4:55 a.m. UTC | #10 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, May 17, 2022 2:52 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wu, Wenjun1 <wenjun1.wu@intel.com>; dev@dpdk.org; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v1] net/iavf: fix resource leak issue
> 
> 17/05/2022 02:59, Zhang, Qi Z:
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 12/05/2022 02:20, Zhang, Qi Z:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 11/05/2022 17:31, Zhang, Qi Z:
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > 11/05/2022 13:46, Zhang, Qi Z:
> > > > > > > > > From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > > > > >
> > > > > > > > > > This patch fixes resource leak issue reported by coverity.
> > > > > > > > > >
> > > > > > > > > > Coverity issue: 378017
> > > > > > > > > > Fixes: b14e8a57b9fe ("net/iavf: support quanta size
> > > > > > > > > > configuration")
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> > > > > > > > >
> > > > > > > > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > > > > > > > >
> > > > > > > > > Applied to dpdk-next-net-intel.
> > > > > > > >
> > > > > > > > In the title, the word "issue" is useless.
> > > > > > > > It would be more interesting to precise where is the leak.
> > > > > > > >
> > > > > > >
> > > > > > > Added below into commit log in dpdk-next-net-intel.
> > > > > > >
> > > > > > > kvlist should be freed before return an error due to invalid quanta
> size.
> > > > > >
> > > > > > It does not explain what is the scenario. What is the impacted feature?
> > > > >
> > > > > I think the scenario has been explained as
> > > > >
> > > > > "kvlist should be freed before return an error due to invalid quanta size"
> > > > > the impact has been mentioned as "resource leak" I guess this is
> > > > > how Coverity describe the issue
> > > >
> > > > No, it does not say how to reproduce (the scenario).
> > > > We have no idea where it is in the code.
> > >
> > > set an invalid quanta size from devargs will reproduce the issue
> > > >
> > > >
> > Refine the Title / commit log in dpdk-next-net-intel as below to
> > answer Thomas's questions
> >
> > net/iavf: fix memory leak issue
> 
> A memory leak is an issue for sure, so no need to add "issue".
> 
> > Set an invalid quanta size from devargs will cause memory leak and
> > this
> 
> Set -> Setting
> 
> > is reported by coverity.
> >
> > The patch fix the issue by correcting the error handle.
> 
> handle -> handling
> 
> Thanks
> 

Ok fixed grammar in dpdk-next-net-intel as suggested.
diff mbox series

Patch

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index d1a2b53675..82672841f4 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -2188,7 +2188,8 @@  static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	    ad->devargs.quanta_size & 0x40) {
 		PMD_INIT_LOG(ERR, "invalid quanta size\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto bail;
 	}
 
 bail: