[PATCH] selftests: net: avoid memory leak

Zongmin Zhou posted 1 patch 1 month, 1 week ago
There is a newer version of this series
tools/testing/selftests/net/cmsg_sender.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] selftests: net: avoid memory leak
Posted by Zongmin Zhou 1 month, 1 week ago
From: Zongmin Zhou <zhouzongmin@kylinos.cn>

The buffer be used without free,fix it to avoid memory leak.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 tools/testing/selftests/net/cmsg_sender.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
index a825e628aee7..5358aa09ecb9 100644
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -491,6 +491,7 @@ int main(int argc, char *argv[])
 	if (err) {
 		fprintf(stderr, "Can't resolve address [%s]:%s\n",
 			opt.host, opt.service);
+		free(buf);
 		return ERN_SOCK_CREATE;
 	}
 
@@ -501,6 +502,7 @@ int main(int argc, char *argv[])
 	if (fd < 0) {
 		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
 		freeaddrinfo(ai);
+		free(buf);
 		return ERN_RESOLVE;
 	}
 
@@ -575,5 +577,6 @@ int main(int argc, char *argv[])
 err_out:
 	close(fd);
 	freeaddrinfo(ai);
+	free(buf);
 	return err;
 }
-- 
2.34.1
Re: [PATCH] selftests: net: avoid memory leak
Posted by Simon Horman 1 month, 1 week ago
On Tue, Aug 26, 2025 at 11:15:40AM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> The buffer be used without free,fix it to avoid memory leak.

I'm assuming this is a short-lived user-space program.
And any memory is freed when it exits.
So I'm unsure about the value of this change.

> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
>  tools/testing/selftests/net/cmsg_sender.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
> index a825e628aee7..5358aa09ecb9 100644
> --- a/tools/testing/selftests/net/cmsg_sender.c
> +++ b/tools/testing/selftests/net/cmsg_sender.c
> @@ -491,6 +491,7 @@ int main(int argc, char *argv[])
>  	if (err) {
>  		fprintf(stderr, "Can't resolve address [%s]:%s\n",
>  			opt.host, opt.service);
> +		free(buf);
>  		return ERN_SOCK_CREATE;
>  	}
>  
> @@ -501,6 +502,7 @@ int main(int argc, char *argv[])
>  	if (fd < 0) {
>  		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
>  		freeaddrinfo(ai);
> +		free(buf);
>  		return ERN_RESOLVE;
>  	}
>  
> @@ -575,5 +577,6 @@ int main(int argc, char *argv[])
>  err_out:
>  	close(fd);
>  	freeaddrinfo(ai);

I think it would be nicer to add another label here, say err_free_buff,
and then use it in the previous two hunks.

> +	free(buf);
>  	return err;
>  }
> -- 
> 2.34.1
>
[PATCH v2] selftests: net: avoid memory leak
Posted by Zongmin Zhou 1 month ago
From: Zongmin Zhou <zhouzongmin@kylinos.cn>

The buffer be used without free,fix it to avoid memory leak.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
Changes in v2:
- add the label to use instead of directly to use on each case.
---
 tools/testing/selftests/net/cmsg_sender.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
index a825e628aee7..71984e0a44e2 100644
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -491,7 +491,8 @@ int main(int argc, char *argv[])
 	if (err) {
 		fprintf(stderr, "Can't resolve address [%s]:%s\n",
 			opt.host, opt.service);
-		return ERN_SOCK_CREATE;
+		err = ERN_SOCK_CREATE;
+		goto err_free_buff;
 	}
 
 	if (ai->ai_family == AF_INET6 && opt.sock.proto == IPPROTO_ICMP)
@@ -501,7 +502,8 @@ int main(int argc, char *argv[])
 	if (fd < 0) {
 		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
 		freeaddrinfo(ai);
-		return ERN_RESOLVE;
+		err = ERN_RESOLVE;
+		goto err_free_buff;
 	}
 
 	if (opt.sock.proto == IPPROTO_ICMP) {
@@ -575,5 +577,7 @@ int main(int argc, char *argv[])
 err_out:
 	close(fd);
 	freeaddrinfo(ai);
+err_free_buff:
+	free(buf);
 	return err;
 }
-- 
2.34.1
Re: [PATCH v2] selftests: net: avoid memory leak
Posted by Jakub Kicinski 1 month ago
On Thu, 28 Aug 2025 10:02:10 +0800 Zongmin Zhou wrote:
> @@ -501,7 +502,8 @@ int main(int argc, char *argv[])
>  	if (fd < 0) {
>  		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
>  		freeaddrinfo(ai);

Since you added the gotos now perhaps it'd be even better to remove
this freeaddrinfo() call here, and instead jump to a separate label...

> -		return ERN_RESOLVE;
> +		err = ERN_RESOLVE;
> +		goto err_free_buff;
>  	}
>  
>  	if (opt.sock.proto == IPPROTO_ICMP) {
> @@ -575,5 +577,7 @@ int main(int argc, char *argv[])
>  err_out:
>  	close(fd);

... added right here?

>  	freeaddrinfo(ai);
> +err_free_buff:
> +	free(buf);
>  	return err;
[PATCH v3] selftests: net: avoid memory leak
Posted by Zongmin Zhou 1 month ago
From: Zongmin Zhou <zhouzongmin@kylinos.cn>

The buffer be used without free,fix it to avoid memory leak.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
Changes in v3:
- move freeaddrinfo() to a separate label.
Changes in v2:
- add the label to use instead of directly to use on each case.
---
 tools/testing/selftests/net/cmsg_sender.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c
index a825e628aee7..ded9b925865e 100644
--- a/tools/testing/selftests/net/cmsg_sender.c
+++ b/tools/testing/selftests/net/cmsg_sender.c
@@ -491,7 +491,8 @@ int main(int argc, char *argv[])
 	if (err) {
 		fprintf(stderr, "Can't resolve address [%s]:%s\n",
 			opt.host, opt.service);
-		return ERN_SOCK_CREATE;
+		err = ERN_SOCK_CREATE;
+		goto err_free_buff;
 	}
 
 	if (ai->ai_family == AF_INET6 && opt.sock.proto == IPPROTO_ICMP)
@@ -500,8 +501,8 @@ int main(int argc, char *argv[])
 	fd = socket(ai->ai_family, opt.sock.type, opt.sock.proto);
 	if (fd < 0) {
 		fprintf(stderr, "Can't open socket: %s\n", strerror(errno));
-		freeaddrinfo(ai);
-		return ERN_RESOLVE;
+		err = ERN_RESOLVE;
+		goto err_free_info;
 	}
 
 	if (opt.sock.proto == IPPROTO_ICMP) {
@@ -574,6 +575,9 @@ int main(int argc, char *argv[])
 
 err_out:
 	close(fd);
+err_free_info:
 	freeaddrinfo(ai);
+err_free_buff:
+	free(buf);
 	return err;
 }
-- 
2.34.1
Re: [PATCH v3] selftests: net: avoid memory leak
Posted by Simon Horman 1 month ago
On Mon, Sep 01, 2025 at 01:45:57PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> The buffer be used without free,fix it to avoid memory leak.
> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
> Changes in v3:
> - move freeaddrinfo() to a separate label.
> Changes in v2:
> - add the label to use instead of directly to use on each case.

Reviewed-by: Simon Horman <horms@kernel.org>
Re: [PATCH v2] selftests: net: avoid memory leak
Posted by Simon Horman 1 month ago
On Thu, Aug 28, 2025 at 10:02:10AM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> The buffer be used without free,fix it to avoid memory leak.
> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
> Changes in v2:
> - add the label to use instead of directly to use on each case.

Thanks for the update.

Reviewed-by: Simon Horman <horms@kernel.org>