[dpdk-dev] [PATCH] net/failsafe: fix resource leak on parse error

[dpdk-dev] [PATCH] net/failsafe: fix resource leak on parse error

[Gaetan Rivet]

If fgets fails, the handle fp is not properly closed.

Coverity issue: 158633
Fixes: a0194d828100 ("net/failsafe: add flexible device definition")

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/net/failsafe/failsafe_args.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 932e371..3f6430b 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -145,22 +145,20 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline)
 	/* We only read one line */
 	if (fgets(output, sizeof(output) - 1, fp) == NULL) {
 		DEBUG("Could not read command output");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto ret_pclose;
 	}
 	fs_sanitize_cmdline(output);
 	ret = fs_parse_device(sdev, output);
-	if (ret) {
+	if (ret)
 		ERROR("Parsing device '%s' failed", output);
-		goto ret_pclose;
-	}
 ret_pclose:
-	ret = pclose(fp);
-	if (ret) {
-		ret = errno;
+	if (pclose(fp)) {
+		if (ret == 0)
+			ret = errno;
 		ERROR("pclose: %s", strerror(errno));
-		errno = old_err;
-		return ret;
 	}
+	errno = old_err;
 	return ret;
 }
 
-- 
2.1.4

Re: [dpdk-dev] [PATCH] net/failsafe: fix resource leak on parse error

[Gaëtan Rivet]

self-nack.

This issue has been fixed by Raslan in [1].

[1]: http://dpdk.org/ml/archives/dev/2017-August/072429.html

On Thu, Aug 03, 2017 at 02:34:00PM +0200, Gaetan Rivet wrote:
> If fgets fails, the handle fp is not properly closed.
> 
> Coverity issue: 158633
> Fixes: a0194d828100 ("net/failsafe: add flexible device definition")
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  drivers/net/failsafe/failsafe_args.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
> index 932e371..3f6430b 100644
> --- a/drivers/net/failsafe/failsafe_args.c
> +++ b/drivers/net/failsafe/failsafe_args.c
> @@ -145,22 +145,20 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline)
>  	/* We only read one line */
>  	if (fgets(output, sizeof(output) - 1, fp) == NULL) {
>  		DEBUG("Could not read command output");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto ret_pclose;
>  	}
>  	fs_sanitize_cmdline(output);
>  	ret = fs_parse_device(sdev, output);
> -	if (ret) {
> +	if (ret)
>  		ERROR("Parsing device '%s' failed", output);
> -		goto ret_pclose;
> -	}
>  ret_pclose:
> -	ret = pclose(fp);
> -	if (ret) {
> -		ret = errno;
> +	if (pclose(fp)) {
> +		if (ret == 0)
> +			ret = errno;
>  		ERROR("pclose: %s", strerror(errno));
> -		errno = old_err;
> -		return ret;
>  	}
> +	errno = old_err;
>  	return ret;
>  }
>  
> -- 
> 2.1.4
> 

-- 
Gaëtan Rivet
6WIND