1. Patchew
  2. linux
  3. View series

[PATCH v2] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()

Malaya Kumar Rout posted 1 patch 10 months ago
There is a newer version of this series
tools/testing/selftests/x86/lam.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
[PATCH v2] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Malaya Kumar Rout 10 months ago 
Exception branch returns without closing
the file descriptors 'file_fd' and 'fd'

Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
:wq
---
 tools/testing/selftests/x86/lam.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
index 18d736640ece..1d3631ce4b69 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
 		return 1;
 
 	if (fstat(file_fd, &st) < 0)
-		return 1;
+		goto cleanup;
 
 	off_t file_sz = st.st_size;
 
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
 
 	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
 	if (!fi)
-		return 1;
+		goto cleanup;
 
 	fi->file_sz = file_sz;
 	fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
 	ring = malloc(sizeof(*ring));
 	if (!ring) {
 		free(fi);
-		return 1;
+		goto cleanup;
 	}
 
 	memset(ring, 0, sizeof(struct io_ring));
@@ -729,6 +729,8 @@ int do_uring(unsigned long lam)
 	}
 
 	free(fi);
+cleanup:
+	close(file_fd);
 
 	return ret;
 }
@@ -1189,9 +1191,10 @@ void *allocate_dsa_pasid(void)
 
 	wq = mmap(NULL, 0x1000, PROT_WRITE,
 			   MAP_SHARED | MAP_POPULATE, fd, 0);
-	if (wq == MAP_FAILED)
+	if (wq == MAP_FAILED){
+		close(fd);
 		perror("mmap");
-
+	}
 	return wq;
 }
 
-- 
2.43.0
Re: [PATCH v2] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Ingo Molnar 10 months ago 
* Malaya Kumar Rout <malayarout91@gmail.com> wrote:

> @@ -1189,9 +1191,10 @@ void *allocate_dsa_pasid(void)
>  
>  	wq = mmap(NULL, 0x1000, PROT_WRITE,
>  			   MAP_SHARED | MAP_POPULATE, fd, 0);
> -	if (wq == MAP_FAILED)
> +	if (wq == MAP_FAILED){
> +		close(fd);
>  		perror("mmap");

We should unconditionally close 'fd' after the mmap() call, not just in 
the perror() branch.

Thanks,

	Ingo
[PATCH v3] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Malaya Kumar Rout 10 months ago 
Exception branch returns without closing
the file descriptors 'file_fd' and 'fd'

Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
---
 tools/testing/selftests/x86/lam.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
index 18d736640ece..88482d8112de 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
 		return 1;
 
 	if (fstat(file_fd, &st) < 0)
-		return 1;
+		goto cleanup;
 
 	off_t file_sz = st.st_size;
 
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
 
 	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
 	if (!fi)
-		return 1;
+		goto cleanup;
 
 	fi->file_sz = file_sz;
 	fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
 	ring = malloc(sizeof(*ring));
 	if (!ring) {
 		free(fi);
-		return 1;
+		goto cleanup;
 	}
 
 	memset(ring, 0, sizeof(struct io_ring));
@@ -729,6 +729,8 @@ int do_uring(unsigned long lam)
 	}
 
 	free(fi);
+cleanup:
+	close(file_fd);
 
 	return ret;
 }
@@ -1192,6 +1194,7 @@ void *allocate_dsa_pasid(void)
 	if (wq == MAP_FAILED)
 		perror("mmap");
 
+	close(fd);
 	return wq;
 }
 
-- 
2.43.0
Re: [PATCH v3] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Ingo Molnar 10 months ago 
* Malaya Kumar Rout <malayarout91@gmail.com> wrote:

> Exception branch returns without closing
> the file descriptors 'file_fd' and 'fd'
> 
> Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
> ---
>  tools/testing/selftests/x86/lam.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
> index 18d736640ece..88482d8112de 100644
> --- a/tools/testing/selftests/x86/lam.c
> +++ b/tools/testing/selftests/x86/lam.c
> @@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
>  		return 1;
>  
>  	if (fstat(file_fd, &st) < 0)
> -		return 1;
> +		goto cleanup;
>  
>  	off_t file_sz = st.st_size;
>  
> @@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
>  
>  	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
>  	if (!fi)
> -		return 1;
> +		goto cleanup;
>  
>  	fi->file_sz = file_sz;
>  	fi->file_fd = file_fd;
> @@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
>  	ring = malloc(sizeof(*ring));
>  	if (!ring) {
>  		free(fi);
> -		return 1;
> +		goto cleanup;
>  	}
>  
>  	memset(ring, 0, sizeof(struct io_ring));
> @@ -729,6 +729,8 @@ int do_uring(unsigned long lam)
>  	}
>  
>  	free(fi);
> +cleanup:
> +	close(file_fd);
>  
>  	return ret;
>  }
> @@ -1192,6 +1194,7 @@ void *allocate_dsa_pasid(void)
>  	if (wq == MAP_FAILED)
>  		perror("mmap");
>  
> +	close(fd);
>  	return wq;

So in your previous patch you closed the file before the perror(), 
presumably so that file-leak detection in Valgrind or whatever tool you 
are using doesn't trigger.

But here it's done after the perror() call, why? It's perfectly fine to 
close the mapping fd straight after an mmap() call.

Finally, it would be nice to quote the before/after output of the leak 
detection tool you are using.

Thanks,

	Ingo
RE:[PATCH RESEND x86-next v4] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Malaya Kumar Rout 10 months ago 
Exception branch returns without closing
the file descriptors 'file_fd' and 'fd'

Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
---
 tools/testing/selftests/x86/lam.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
index 18d736640ece..0873b0e5f48b 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
 		return 1;
 
 	if (fstat(file_fd, &st) < 0)
-		return 1;
+		goto cleanup;
 
 	off_t file_sz = st.st_size;
 
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
 
 	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
 	if (!fi)
-		return 1;
+		goto cleanup;
 
 	fi->file_sz = file_sz;
 	fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
 	ring = malloc(sizeof(*ring));
 	if (!ring) {
 		free(fi);
-		return 1;
+		goto cleanup;
 	}
 
 	memset(ring, 0, sizeof(struct io_ring));
@@ -729,6 +729,8 @@ int do_uring(unsigned long lam)
 	}
 
 	free(fi);
+cleanup:
+	close(file_fd);
 
 	return ret;
 }
@@ -1189,6 +1191,7 @@ void *allocate_dsa_pasid(void)
 
 	wq = mmap(NULL, 0x1000, PROT_WRITE,
 			   MAP_SHARED | MAP_POPULATE, fd, 0);
+	close(fd);
 	if (wq == MAP_FAILED)
 		perror("mmap");
 
-- 
2.43.0
Re: [PATCH RESEND x86-next v4] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Ingo Molnar 10 months ago 
* Malaya Kumar Rout <malayarout91@gmail.com> wrote:

> Exception branch returns without closing
> the file descriptors 'file_fd' and 'fd'
> 
> Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
> ---
>  tools/testing/selftests/x86/lam.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Why have you ignored this request I made:

>> Finally, it would be nice to quote the before/after output of the
>> leak detection tool you are using.

?

Thanks,

	Ingo
[PATCH v4] selftests/x86/lam: fix resource leak in do_uring() and allocate_dsa_pasid()
Posted by Malaya Kumar Rout 10 months ago 
This patch resolves resource leaks reported by cppcheck in lam.c.
Specifically, the 'file_fd' and 'fd' were not closed in do_uring()
and allocate_dsa_pasid() functions, respectively.

cppcheck output before this patch:
tools/testing/selftests/x86/lam.c:685:3: error: Resource leak: file_fd [resourceLeak]
tools/testing/selftests/x86/lam.c:693:3: error: Resource leak: file_fd [resourceLeak]
tools/testing/selftests/x86/lam.c:1195:2: error: Resource leak: fd [resourceLeak]

cppcheck output after this patch:
No resource leaks found

Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
---
 tools/testing/selftests/x86/lam.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
index 18d736640ece..0873b0e5f48b 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
 		return 1;
 
 	if (fstat(file_fd, &st) < 0)
-		return 1;
+		goto cleanup;
 
 	off_t file_sz = st.st_size;
 
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
 
 	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
 	if (!fi)
-		return 1;
+		goto cleanup;
 
 	fi->file_sz = file_sz;
 	fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
 	ring = malloc(sizeof(*ring));
 	if (!ring) {
 		free(fi);
-		return 1;
+		goto cleanup;
 	}
 
 	memset(ring, 0, sizeof(struct io_ring));
@@ -729,6 +729,8 @@ int do_uring(unsigned long lam)
 	}
 
 	free(fi);
+cleanup:
+	close(file_fd);
 
 	return ret;
 }
@@ -1189,6 +1191,7 @@ void *allocate_dsa_pasid(void)
 
 	wq = mmap(NULL, 0x1000, PROT_WRITE,
 			   MAP_SHARED | MAP_POPULATE, fd, 0);
+	close(fd);
 	if (wq == MAP_FAILED)
 		perror("mmap");
 
-- 
2.43.0
[tip: x86/mm] selftests/x86/lam: Fix clean up fds in do_uring() and allocate_dsa_pasid()
Posted by tip-bot2 for Malaya Kumar Rout 10 months ago 
The following commit has been merged into the x86/mm branch of tip:

Commit-ID:     60567e93c05d7064c93830cf4bf0d2c58f11b2f2
Gitweb:        https://git.kernel.org/tip/60567e93c05d7064c93830cf4bf0d2c58f11b2f2
Author:        Malaya Kumar Rout <malayarout91@gmail.com>
AuthorDate:    Wed, 09 Apr 2025 19:23:37 +05:30
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 09 Apr 2025 21:30:37 +02:00

selftests/x86/lam: Fix clean up fds in do_uring() and allocate_dsa_pasid()

Resolve minor fd leaks reported by cppcheck in lam.c.

Specifically, the 'file_fd' and 'fd' were not closed in do_uring()
and allocate_dsa_pasid() functions, respectively.

cppcheck output before this patch:

  tools/testing/selftests/x86/lam.c:685:3: error: Resource leak: file_fd [resourceLeak]
  tools/testing/selftests/x86/lam.c:693:3: error: Resource leak: file_fd [resourceLeak]
  tools/testing/selftests/x86/lam.c:1195:2: error: Resource leak: fd [resourceLeak]

cppcheck output after this patch:

  No resource leaks found

While this is a standalone test tool that doesn't really leak anything
in practice, as exit() cleans it up all, clean up resources nevertheless.

[ mingo: Updated the changelog. ]

Signed-off-by: Malaya Kumar Rout <malayarout91@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20250409135341.28987-1-malayarout91@gmail.com
---
 tools/testing/selftests/x86/lam.c |  9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/x86/lam.c b/tools/testing/selftests/x86/lam.c
index 18d7366..0873b0e 100644
--- a/tools/testing/selftests/x86/lam.c
+++ b/tools/testing/selftests/x86/lam.c
@@ -682,7 +682,7 @@ int do_uring(unsigned long lam)
 		return 1;
 
 	if (fstat(file_fd, &st) < 0)
-		return 1;
+		goto cleanup;
 
 	off_t file_sz = st.st_size;
 
@@ -690,7 +690,7 @@ int do_uring(unsigned long lam)
 
 	fi = malloc(sizeof(*fi) + sizeof(struct iovec) * blocks);
 	if (!fi)
-		return 1;
+		goto cleanup;
 
 	fi->file_sz = file_sz;
 	fi->file_fd = file_fd;
@@ -698,7 +698,7 @@ int do_uring(unsigned long lam)
 	ring = malloc(sizeof(*ring));
 	if (!ring) {
 		free(fi);
-		return 1;
+		goto cleanup;
 	}
 
 	memset(ring, 0, sizeof(struct io_ring));
@@ -729,6 +729,8 @@ out:
 	}
 
 	free(fi);
+cleanup:
+	close(file_fd);
 
 	return ret;
 }
@@ -1189,6 +1191,7 @@ void *allocate_dsa_pasid(void)
 
 	wq = mmap(NULL, 0x1000, PROT_WRITE,
 			   MAP_SHARED | MAP_POPULATE, fd, 0);
+	close(fd);
 	if (wq == MAP_FAILED)
 		perror("mmap");

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.