metrics: fix resource leak on memory allocation fail

Message ID 20200917150341.66298-1-ciara.power@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series metrics: fix resource leak on memory allocation fail |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-testing success Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Power, Ciara Sept. 17, 2020, 3:03 p.m. UTC
  If an error occurred when allocating memory for metrics or names,
the function returned without freeing allocated memory. This is now
fixed to avoid the resource leak in the case that either metrics or
names had been successfully allocated memory.

Coverity issue: 362053
Fixes: c5b7197f662e ("telemetry: move some functions to metrics library")
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_metrics/rte_metrics_telemetry.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

John McNamara Sept. 17, 2020, 4:08 p.m. UTC | #1
> If an error occurred when allocating memory for metrics or names, the
> function returned without freeing allocated memory. This is now fixed to
> avoid the resource leak in the case that either metrics or names had been
> successfully allocated memory.
> 
> Coverity issue: 362053
> Fixes: c5b7197f662e ("telemetry: move some functions to metrics library")
> Cc: stable@dpdk.org

Acked-by: John McNamara <john.mcnamara@intel.com>
  
Honnappa Nagarahalli Sept. 21, 2020, 11:43 p.m. UTC | #2
<snip>

> 
> > If an error occurred when allocating memory for metrics or names, the
> > function returned without freeing allocated memory. This is now fixed
> > to avoid the resource leak in the case that either metrics or names
> > had been successfully allocated memory.
> >
> > Coverity issue: 362053
> > Fixes: c5b7197f662e ("telemetry: move some functions to metrics
> > library")
> > Cc: stable@dpdk.org
> 
> Acked-by: John McNamara <john.mcnamara@intel.com>
> 
This is fixed in https://patches.dpdk.org/patch/75323/. Can you please check if that suffices?
  
Power, Ciara Sept. 22, 2020, 10:28 a.m. UTC | #3
Hi Honnappa,

>-----Original Message-----
>From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>Sent: Tuesday 22 September 2020 00:43
>To: Mcnamara, John <john.mcnamara@intel.com>; Power, Ciara
><ciara.power@intel.com>; dev@dpdk.org
>Cc: Power, Ciara <ciara.power@intel.com>; stable@dpdk.org; Wiles, Keith
><keith.wiles@intel.com>; nd <nd@arm.com>; Honnappa Nagarahalli
><Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
>Subject: RE: [dpdk-dev] [dpdk-stable] [PATCH] metrics: fix resource leak on
>memory allocation fail
>
><snip>
>
>>
>> > If an error occurred when allocating memory for metrics or names,
>> > the function returned without freeing allocated memory. This is now
>> > fixed to avoid the resource leak in the case that either metrics or
>> > names had been successfully allocated memory.
>> >
>> > Coverity issue: 362053
>> > Fixes: c5b7197f662e ("telemetry: move some functions to metrics
>> > library")
>> > Cc: stable@dpdk.org
>>
>> Acked-by: John McNamara <john.mcnamara@intel.com>
>>
>This is fixed in https://patches.dpdk.org/patch/75323/. Can you please check
>if that suffices?

I left some comments on that patch, but seeing as it is fixing the same issue, I will remove this patch.

Thanks,
Ciara
  
David Marchand Nov. 3, 2020, 9:36 p.m. UTC | #4
On Thu, Sep 17, 2020 at 5:06 PM Ciara Power <ciara.power@intel.com> wrote:
>
> If an error occurred when allocating memory for metrics or names,
> the function returned without freeing allocated memory. This is now
> fixed to avoid the resource leak in the case that either metrics or
> names had been successfully allocated memory.
>
> Coverity issue: 362053
> Fixes: c5b7197f662e ("telemetry: move some functions to metrics library")
> Cc: stable@dpdk.org
>

Gaurav reported the same issue and proposed a patch, but I went with
yours as it describes the issue, is minimal and has the proper tags
for backport.

Reported-by: Gaurav Singh <gaurav1086@gmail.com>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>


Applied, thanks.
  

Patch

diff --git a/lib/librte_metrics/rte_metrics_telemetry.c b/lib/librte_metrics/rte_metrics_telemetry.c
index 289ebae0bd..36a3821b4c 100644
--- a/lib/librte_metrics/rte_metrics_telemetry.c
+++ b/lib/librte_metrics/rte_metrics_telemetry.c
@@ -170,7 +170,8 @@  rte_metrics_tel_format_port(uint32_t pid, json_t *ports,
 	names = malloc(sizeof(struct rte_metric_name) * num_metrics);
 	if (metrics == NULL || names == NULL) {
 		METRICS_LOG_ERR("Cannot allocate memory");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto fail;
 	}
 
 	if (rte_metrics_get_names(names, num_metrics) != num_metrics ||