[PATCH] perf test: Avoid shell test description infinite loop

[PATCH] perf test: Avoid shell test description infinite loop

[Ian Rogers]

for_each_shell_test is already strict in expecting tests to be files and
executable. It is sometimes possible when it iterates over all files
that it finds one that is executable and lacks a newline character. When
this happens the loop never terminates as it doesn't check for EOF. Add
the EOF check to make this loop at least bounded by the file size.

If the description is returned as NULL then also skip the test.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/builtin-test.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 3c34cb766724..aa40eae1c9cf 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -279,6 +279,7 @@ static const char *shell_test__description(char *description, size_t size,
 {
 	FILE *fp;
 	char filename[PATH_MAX];
+	int ch;
 
 	path__join(filename, sizeof(filename), path, name);
 	fp = fopen(filename, "r");
@@ -286,7 +287,9 @@ static const char *shell_test__description(char *description, size_t size,
 		return NULL;
 
 	/* Skip shebang */
-	while (fgetc(fp) != '\n');
+	do {
+		ch = fgetc(fp);
+	} while (ch != EOF && ch != '\n');
 
 	description = fgets(description, size, fp);
 	fclose(fp);
@@ -419,7 +422,8 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
 			.priv = &st,
 		};
 
-		if (!perf_test__matches(test_suite.desc, curr, argc, argv))
+		if (test_suite.desc == NULL ||
+		    !perf_test__matches(test_suite.desc, curr, argc, argv))
 			continue;
 
 		st.file = ent->d_name;
-- 
2.36.0.550.gb090851708-goog

Re: [PATCH] perf test: Avoid shell test description infinite loop

[Arnaldo Carvalho de Melo]

Em Tue, May 17, 2022 at 01:41:44PM -0700, Ian Rogers escreveu:
> for_each_shell_test is already strict in expecting tests to be files and
> executable. It is sometimes possible when it iterates over all files
> that it finds one that is executable and lacks a newline character. When
> this happens the loop never terminates as it doesn't check for EOF. Add
> the EOF check to make this loop at least bounded by the file size.
> 
> If the description is returned as NULL then also skip the test.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/builtin-test.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 3c34cb766724..aa40eae1c9cf 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -279,6 +279,7 @@ static const char *shell_test__description(char *description, size_t size,
>  {
>  	FILE *fp;
>  	char filename[PATH_MAX];
> +	int ch;
>  
>  	path__join(filename, sizeof(filename), path, name);
>  	fp = fopen(filename, "r");
> @@ -286,7 +287,9 @@ static const char *shell_test__description(char *description, size_t size,
>  		return NULL;
>  
>  	/* Skip shebang */
> -	while (fgetc(fp) != '\n');
> +	do {
> +		ch = fgetc(fp);
> +	} while (ch != EOF && ch != '\n');
>  
>  	description = fgets(description, size, fp);
>  	fclose(fp);
> @@ -419,7 +422,8 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
>  			.priv = &st,
>  		};
>  
> -		if (!perf_test__matches(test_suite.desc, curr, argc, argv))
> +		if (test_suite.desc == NULL ||
> +		    !perf_test__matches(test_suite.desc, curr, argc, argv))
>  			continue;
>  
>  		st.file = ent->d_name;
> -- 
> 2.36.0.550.gb090851708-goog

-- 

- Arnaldo