[PATCH RESEND] perf test pmu: Fix file Leak in test_format_dir_get

Samasth Norway Ananda posted 1 patch 1 year, 7 months ago
tools/perf/tests/pmu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH RESEND] perf test pmu: Fix file Leak in test_format_dir_get
Posted by Samasth Norway Ananda 1 year, 7 months ago
File is opened inside the for loop. But if the 'if' condition is
successful then 'break' statement will be reached, exiting the
'for' loop prior to reaching 'fclose'.

Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
Found this error through static analysis. This has only been compile
tested.
---
 tools/perf/tests/pmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 8f18127d876a..f751e6cb6ac0 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -106,8 +106,10 @@ static char *test_format_dir_get(char *dir, size_t sz)
 		if (!file)
 			return NULL;
 
-		if (1 != fwrite(format->value, strlen(format->value), 1, file))
+		if (1 != fwrite(format->value, strlen(format->value), 1, file)) {
+			fclose(file);
 			break;
+		}
 
 		fclose(file);
 	}
-- 
2.43.0
Re: [PATCH RESEND] perf test pmu: Fix file Leak in test_format_dir_get
Posted by Ian Rogers 1 year, 7 months ago
On Wed, May 15, 2024 at 11:27 AM Samasth Norway Ananda
<samasth.norway.ananda@oracle.com> wrote:
>
> File is opened inside the for loop. But if the 'if' condition is
> successful then 'break' statement will be reached, exiting the
> 'for' loop prior to reaching 'fclose'.
>
> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
> ---
> Found this error through static analysis. This has only been compile
> tested.

Thanks Samasth, I agree with the fix however this code was recently
deleted and isn't in our next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/pmu.c?h=perf-tools-next
the change that removed the code is:
https://lore.kernel.org/all/20240502213507.2339733-4-irogers@google.com/

Thanks,
Ian

> ---
>  tools/perf/tests/pmu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
> index 8f18127d876a..f751e6cb6ac0 100644
> --- a/tools/perf/tests/pmu.c
> +++ b/tools/perf/tests/pmu.c
> @@ -106,8 +106,10 @@ static char *test_format_dir_get(char *dir, size_t sz)
>                 if (!file)
>                         return NULL;
>
> -               if (1 != fwrite(format->value, strlen(format->value), 1, file))
> +               if (1 != fwrite(format->value, strlen(format->value), 1, file)) {
> +                       fclose(file);
>                         break;
> +               }
>
>                 fclose(file);
>         }
> --
> 2.43.0
>