* [dpdk-dev] [PATCH] eal: fix resource leak
@ 2017-09-19 13:34 Daniel Mrzyglod
2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Mrzyglod @ 2017-09-19 13:34 UTC (permalink / raw)
To: thomas; +Cc: dev, Daniel Mrzyglod
Memory allocated in strdup is not free.
Coverity issue: 143257
Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id")
Cc: thomas@monjalon.net
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
lib/librte_eal/linuxapp/eal/eal.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 48f12f4..1bdbc3f 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -781,6 +781,7 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_cpu_init() < 0) {
rte_eal_init_alert("Cannot detect lcores.");
rte_errno = ENOTSUP;
+ free(logid);
return -1;
}
@@ -789,6 +790,7 @@ rte_eal_init(int argc, char **argv)
rte_eal_init_alert("Invalid 'command line' arguments.");
rte_errno = EINVAL;
rte_atomic32_clear(&run_once);
+ free(logid);
return -1;
}
@@ -799,6 +801,7 @@ rte_eal_init(int argc, char **argv)
rte_eal_init_alert("Cannot get hugepage information.");
rte_errno = EACCES;
rte_atomic32_clear(&run_once);
+ free(logid);
return -1;
}
@@ -826,6 +829,7 @@ rte_eal_init(int argc, char **argv)
rte_eal_init_alert("Cannot init logging.");
rte_errno = ENOMEM;
rte_atomic32_clear(&run_once);
+ free(logid);
return -1;
}
@@ -834,6 +838,7 @@ rte_eal_init(int argc, char **argv)
rte_eal_init_alert("Cannot init VFIO\n");
rte_errno = EAGAIN;
rte_atomic32_clear(&run_once);
+ free(logid);
return -1;
}
#endif
@@ -841,6 +846,7 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_memory_init() < 0) {
rte_eal_init_alert("Cannot init memory\n");
rte_errno = ENOMEM;
+ free(logid);
return -1;
}
@@ -850,24 +856,28 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_memzone_init() < 0) {
rte_eal_init_alert("Cannot init memzone\n");
rte_errno = ENODEV;
+ free(logid);
return -1;
}
if (rte_eal_tailqs_init() < 0) {
rte_eal_init_alert("Cannot init tail queues for objects\n");
rte_errno = EFAULT;
+ free(logid);
return -1;
}
if (rte_eal_alarm_init() < 0) {
rte_eal_init_alert("Cannot init interrupt-handling thread\n");
/* rte_eal_alarm_init sets rte_errno on failure. */
+ free(logid);
return -1;
}
if (rte_eal_timer_init() < 0) {
rte_eal_init_alert("Cannot init HPET or TSC timers\n");
rte_errno = ENOTSUP;
+ free(logid);
return -1;
}
@@ -886,17 +896,20 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_intr_init() < 0) {
rte_eal_init_alert("Cannot init interrupt-handling thread\n");
+ free(logid);
return -1;
}
if (eal_option_device_parse()) {
rte_errno = ENODEV;
+ free(logid);
return -1;
}
if (rte_bus_scan()) {
rte_eal_init_alert("Cannot scan the buses for devices\n");
rte_errno = ENODEV;
+ free(logid);
return -1;
}
@@ -941,6 +954,7 @@ rte_eal_init(int argc, char **argv)
if (ret) {
rte_eal_init_alert("rte_service_init() failed\n");
rte_errno = ENOEXEC;
+ free(logid);
return -1;
}
@@ -948,6 +962,7 @@ rte_eal_init(int argc, char **argv)
if (rte_bus_probe()) {
rte_eal_init_alert("Cannot probe devices\n");
rte_errno = ENOTSUP;
+ free(logid);
return -1;
}
@@ -957,6 +972,7 @@ rte_eal_init(int argc, char **argv)
ret = rte_service_start_with_defaults();
if (ret < 0 && ret != -ENOTSUP) {
rte_errno = ENOEXEC;
+ free(logid);
return -1;
}
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [dpdk-dev] [PATCH v2] eal: fix resource leak 2017-09-19 13:34 [dpdk-dev] [PATCH] eal: fix resource leak Daniel Mrzyglod @ 2017-09-22 14:48 ` Daniel Mrzyglod 2017-10-02 14:16 ` Jastrzebski, MichalX K ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Daniel Mrzyglod @ 2017-09-22 14:48 UTC (permalink / raw) To: thomas; +Cc: dev, Daniel Mrzyglod Memory allocated in strdup is not free. Coverity issue: 143257 Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") Cc: thomas@monjalon.net Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> --- v2: * Fix due to compilation errors lib/librte_eal/linuxapp/eal/eal.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 48f12f4..a7df566 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -751,7 +751,7 @@ rte_eal_init(int argc, char **argv) int i, fctret, ret; pthread_t thread_id; static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0); - const char *logid; + char *logid; char cpuset[RTE_CPU_AFFINITY_STR_LEN]; char thread_name[RTE_MAX_THREAD_NAME_LEN]; @@ -781,6 +781,7 @@ rte_eal_init(int argc, char **argv) if (rte_eal_cpu_init() < 0) { rte_eal_init_alert("Cannot detect lcores."); rte_errno = ENOTSUP; + free(logid); return -1; } @@ -789,6 +790,7 @@ rte_eal_init(int argc, char **argv) rte_eal_init_alert("Invalid 'command line' arguments."); rte_errno = EINVAL; rte_atomic32_clear(&run_once); + free(logid); return -1; } @@ -799,6 +801,7 @@ rte_eal_init(int argc, char **argv) rte_eal_init_alert("Cannot get hugepage information."); rte_errno = EACCES; rte_atomic32_clear(&run_once); + free(logid); return -1; } @@ -826,6 +829,7 @@ rte_eal_init(int argc, char **argv) rte_eal_init_alert("Cannot init logging."); rte_errno = ENOMEM; rte_atomic32_clear(&run_once); + free(logid); return -1; } @@ -834,6 +838,7 @@ rte_eal_init(int argc, char **argv) rte_eal_init_alert("Cannot init VFIO\n"); rte_errno = EAGAIN; rte_atomic32_clear(&run_once); + free(logid); return -1; } #endif @@ -841,6 +846,7 @@ rte_eal_init(int argc, char **argv) if (rte_eal_memory_init() < 0) { rte_eal_init_alert("Cannot init memory\n"); rte_errno = ENOMEM; + free(logid); return -1; } @@ -850,24 +856,28 @@ rte_eal_init(int argc, char **argv) if (rte_eal_memzone_init() < 0) { rte_eal_init_alert("Cannot init memzone\n"); rte_errno = ENODEV; + free(logid); return -1; } if (rte_eal_tailqs_init() < 0) { rte_eal_init_alert("Cannot init tail queues for objects\n"); rte_errno = EFAULT; + free(logid); return -1; } if (rte_eal_alarm_init() < 0) { rte_eal_init_alert("Cannot init interrupt-handling thread\n"); /* rte_eal_alarm_init sets rte_errno on failure. */ + free(logid); return -1; } if (rte_eal_timer_init() < 0) { rte_eal_init_alert("Cannot init HPET or TSC timers\n"); rte_errno = ENOTSUP; + free(logid); return -1; } @@ -886,17 +896,20 @@ rte_eal_init(int argc, char **argv) if (rte_eal_intr_init() < 0) { rte_eal_init_alert("Cannot init interrupt-handling thread\n"); + free(logid); return -1; } if (eal_option_device_parse()) { rte_errno = ENODEV; + free(logid); return -1; } if (rte_bus_scan()) { rte_eal_init_alert("Cannot scan the buses for devices\n"); rte_errno = ENODEV; + free(logid); return -1; } @@ -941,6 +954,7 @@ rte_eal_init(int argc, char **argv) if (ret) { rte_eal_init_alert("rte_service_init() failed\n"); rte_errno = ENOEXEC; + free(logid); return -1; } @@ -948,6 +962,7 @@ rte_eal_init(int argc, char **argv) if (rte_bus_probe()) { rte_eal_init_alert("Cannot probe devices\n"); rte_errno = ENOTSUP; + free(logid); return -1; } @@ -957,6 +972,7 @@ rte_eal_init(int argc, char **argv) ret = rte_service_start_with_defaults(); if (ret < 0 && ret != -ENOTSUP) { rte_errno = ENOEXEC; + free(logid); return -1; } -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: fix resource leak 2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod @ 2017-10-02 14:16 ` Jastrzebski, MichalX K 2017-10-04 19:24 ` Ferruh Yigit 2017-10-11 11:53 ` [dpdk-dev] [PATCH v3] " Daniel Mrzyglod 2 siblings, 0 replies; 7+ messages in thread From: Jastrzebski, MichalX K @ 2017-10-02 14:16 UTC (permalink / raw) To: Mrzyglod, DanielX T, thomas; +Cc: dev, Mrzyglod, DanielX T > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod > Sent: Friday, September 22, 2017 4:48 PM > To: thomas@monjalon.net > Cc: dev@dpdk.org; Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com> > Subject: [dpdk-dev] [PATCH v2] eal: fix resource leak > > Memory allocated in strdup is not free. > > Coverity issue: 143257 > Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") > Cc: thomas@monjalon.net > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > --- > v2: > * Fix due to compilation errors > > lib/librte_eal/linuxapp/eal/eal.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > b/lib/librte_eal/linuxapp/eal/eal.c > index 48f12f4..a7df566 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -751,7 +751,7 @@ rte_eal_init(int argc, char **argv) > int i, fctret, ret; > pthread_t thread_id; > static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0); > - const char *logid; > + char *logid; > char cpuset[RTE_CPU_AFFINITY_STR_LEN]; > char thread_name[RTE_MAX_THREAD_NAME_LEN]; > > @@ -781,6 +781,7 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_cpu_init() < 0) { > rte_eal_init_alert("Cannot detect lcores."); > rte_errno = ENOTSUP; > + free(logid); > return -1; > } > > @@ -789,6 +790,7 @@ rte_eal_init(int argc, char **argv) > rte_eal_init_alert("Invalid 'command line' arguments."); > rte_errno = EINVAL; > rte_atomic32_clear(&run_once); > + free(logid); > return -1; > } > > @@ -799,6 +801,7 @@ rte_eal_init(int argc, char **argv) > rte_eal_init_alert("Cannot get hugepage information."); > rte_errno = EACCES; > rte_atomic32_clear(&run_once); > + free(logid); > return -1; > } > > @@ -826,6 +829,7 @@ rte_eal_init(int argc, char **argv) > rte_eal_init_alert("Cannot init logging."); > rte_errno = ENOMEM; > rte_atomic32_clear(&run_once); > + free(logid); > return -1; > } > > @@ -834,6 +838,7 @@ rte_eal_init(int argc, char **argv) > rte_eal_init_alert("Cannot init VFIO\n"); > rte_errno = EAGAIN; > rte_atomic32_clear(&run_once); > + free(logid); > return -1; > } > #endif > @@ -841,6 +846,7 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_memory_init() < 0) { > rte_eal_init_alert("Cannot init memory\n"); > rte_errno = ENOMEM; > + free(logid); > return -1; > } > > @@ -850,24 +856,28 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_memzone_init() < 0) { > rte_eal_init_alert("Cannot init memzone\n"); > rte_errno = ENODEV; > + free(logid); > return -1; > } > > if (rte_eal_tailqs_init() < 0) { > rte_eal_init_alert("Cannot init tail queues for objects\n"); > rte_errno = EFAULT; > + free(logid); > return -1; > } > > if (rte_eal_alarm_init() < 0) { > rte_eal_init_alert("Cannot init interrupt-handling > thread\n"); > /* rte_eal_alarm_init sets rte_errno on failure. */ > + free(logid); > return -1; > } > > if (rte_eal_timer_init() < 0) { > rte_eal_init_alert("Cannot init HPET or TSC timers\n"); > rte_errno = ENOTSUP; > + free(logid); > return -1; > } > > @@ -886,17 +896,20 @@ rte_eal_init(int argc, char **argv) > > if (rte_eal_intr_init() < 0) { > rte_eal_init_alert("Cannot init interrupt-handling > thread\n"); > + free(logid); > return -1; > } > > if (eal_option_device_parse()) { > rte_errno = ENODEV; > + free(logid); > return -1; > } > > if (rte_bus_scan()) { > rte_eal_init_alert("Cannot scan the buses for devices\n"); > rte_errno = ENODEV; > + free(logid); > return -1; > } > > @@ -941,6 +954,7 @@ rte_eal_init(int argc, char **argv) > if (ret) { > rte_eal_init_alert("rte_service_init() failed\n"); > rte_errno = ENOEXEC; > + free(logid); > return -1; > } > > @@ -948,6 +962,7 @@ rte_eal_init(int argc, char **argv) > if (rte_bus_probe()) { > rte_eal_init_alert("Cannot probe devices\n"); > rte_errno = ENOTSUP; > + free(logid); > return -1; > } > > @@ -957,6 +972,7 @@ rte_eal_init(int argc, char **argv) > ret = rte_service_start_with_defaults(); > if (ret < 0 && ret != -ENOTSUP) { > rte_errno = ENOEXEC; > + free(logid); > return -1; > } > > -- > 2.7.4 Hi Thomas, I would like to ask for a feedback regarding proposed fix. If everything is ok with it, please send acked-by. Best regards Michal. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: fix resource leak 2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod 2017-10-02 14:16 ` Jastrzebski, MichalX K @ 2017-10-04 19:24 ` Ferruh Yigit 2017-10-05 22:33 ` Thomas Monjalon 2017-10-11 11:53 ` [dpdk-dev] [PATCH v3] " Daniel Mrzyglod 2 siblings, 1 reply; 7+ messages in thread From: Ferruh Yigit @ 2017-10-04 19:24 UTC (permalink / raw) To: Daniel Mrzyglod, thomas; +Cc: dev On 9/22/2017 3:48 PM, Daniel Mrzyglod wrote: > Memory allocated in strdup is not free. > > Coverity issue: 143257 > Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") > Cc: thomas@monjalon.net > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > --- > v2: > * Fix due to compilation errors > > lib/librte_eal/linuxapp/eal/eal.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c > index 48f12f4..a7df566 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -751,7 +751,7 @@ rte_eal_init(int argc, char **argv) > int i, fctret, ret; > pthread_t thread_id; > static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0); > - const char *logid; > + char *logid; > char cpuset[RTE_CPU_AFFINITY_STR_LEN]; > char thread_name[RTE_MAX_THREAD_NAME_LEN]; > > @@ -781,6 +781,7 @@ rte_eal_init(int argc, char **argv) > if (rte_eal_cpu_init() < 0) { > rte_eal_init_alert("Cannot detect lcores."); > rte_errno = ENOTSUP; > + free(logid); Hi Daniel, This works but this variable is a nuance and adding free() for this it into main eal features fail path looks like noise. Initially, do we need to strdup this variable at all? What will happen if logid fed into rte_eal_log_init() without strdup? Since it is const char *, I guess the string is just for read and content won't be changed so it should be OK I guess. If above is not right, what about creating a static variable and use it instead of dynamically allocating the logid, what do you think? Thanks, ferruh > return -1; > } > > @@ -789,6 +790,7 @@ rte_eal_init(int argc, char **argv) > rte_eal_init_alert("Invalid 'command line' arguments."); > rte_errno = EINVAL; > rte_atomic32_clear(&run_once); > + free(logid); > return -1; > } > <...> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: fix resource leak 2017-10-04 19:24 ` Ferruh Yigit @ 2017-10-05 22:33 ` Thomas Monjalon 0 siblings, 0 replies; 7+ messages in thread From: Thomas Monjalon @ 2017-10-05 22:33 UTC (permalink / raw) To: Ferruh Yigit, Daniel Mrzyglod; +Cc: dev 04/10/2017 21:24, Ferruh Yigit: > On 9/22/2017 3:48 PM, Daniel Mrzyglod wrote: > > Memory allocated in strdup is not free. > > > > Coverity issue: 143257 > > Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") > > Cc: thomas@monjalon.net > > > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > > --- > This works but this variable is a nuance and adding free() for this it > into main eal features fail path looks like noise. > > Initially, do we need to strdup this variable at all? > What will happen if logid fed into rte_eal_log_init() without strdup? > Since it is const char *, I guess the string is just for read and > content won't be changed so it should be OK I guess. > > If above is not right, what about creating a static variable and use it > instead of dynamically allocating the logid, what do you think? Good proposal Ferruh. It seems strdup is not needed as it is basically argv[0]. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v3] eal: fix resource leak 2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod 2017-10-02 14:16 ` Jastrzebski, MichalX K 2017-10-04 19:24 ` Ferruh Yigit @ 2017-10-11 11:53 ` Daniel Mrzyglod 2017-10-11 12:43 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon 2 siblings, 1 reply; 7+ messages in thread From: Daniel Mrzyglod @ 2017-10-11 11:53 UTC (permalink / raw) To: thomas; +Cc: dev, Daniel Mrzyglod, stable Memory allocated in strdup is not free. Coverity issue: 143257 Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") Cc: thomas@monjalon.net Cc: stable@dpdk.org Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> --- v3: * remove strdup because it's basically striped argv[0] v2: * Fix due to compilation errors lib/librte_eal/linuxapp/eal/eal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index 1f07347..739b61a 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -763,7 +763,7 @@ rte_eal_init(int argc, char **argv) } logid = strrchr(argv[0], '/'); - logid = strdup(logid ? logid + 1: argv[0]); + logid = logid ? logid + 1 : argv[0]; thread_id = pthread_self(); -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3] eal: fix resource leak 2017-10-11 11:53 ` [dpdk-dev] [PATCH v3] " Daniel Mrzyglod @ 2017-10-11 12:43 ` Thomas Monjalon 0 siblings, 0 replies; 7+ messages in thread From: Thomas Monjalon @ 2017-10-11 12:43 UTC (permalink / raw) To: Daniel Mrzyglod; +Cc: stable, dev 11/10/2017 13:53, Daniel Mrzyglod: > Memory allocated in strdup is not free. > > Coverity issue: 143257 > Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id") > Cc: thomas@monjalon.net > Cc: stable@dpdk.org > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> Given this recent commit: http://dpdk.org/commit/e3f141879ef I think we should consider this issue as a false positive. The arguments given to rte_eal_init may be freed. So it's better to strdup them, even if it is never freed. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-11 12:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-19 13:34 [dpdk-dev] [PATCH] eal: fix resource leak Daniel Mrzyglod 2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod 2017-10-02 14:16 ` Jastrzebski, MichalX K 2017-10-04 19:24 ` Ferruh Yigit 2017-10-05 22:33 ` Thomas Monjalon 2017-10-11 11:53 ` [dpdk-dev] [PATCH v3] " Daniel Mrzyglod 2017-10-11 12:43 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
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).