[PATCH bpf-next v2 1/3] selftests/bpf: fix for veristat file/prog filters processing

Eduard Zingerman <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
At the moment veristat filtering behaves unexpectedly for the
following filter expression:

  -f !file/prog

The expression rejects all programs with name 'prog', and all programs
in a file with name 'file'. This commit fixes the expression to
exclude only a program 'prog' from a file 'file'. Additionally,
the commit makes empty filters like '-f ""' or '-f "/"' and error.
Here is the filtering behaviour compared old versus new:

| filter   | file | prog | old verdict | new verdict |
|----------+------+------+-------------+-------------|
| !foo     | foo  | bar  | skipped     | skipped     |
| !foo     | bar  | foo  | skipped     | skipped     |
| !foo     | bar  | bar  | processed   | processed   |
| !foo/bar | foo  | bar  | skipped     | skipped     |
| !foo/bar | foo  | buz  | skipped     | processed   | (!)
| !foo/bar | bar  | bar  | skipped     | processed   | (!)
| !foo/    | foo  | bar  | skipped     | skipped     |
| !foo/    | bar  | bar  | processed   | processed   |
| !/bar    | foo  | bar  | skipped     | skipped     |
| !/bar    | foo  | foo  | processed   | processed   |
| !/       | foo  | bar  | processed   | error       | (!)
| !        | foo  | bar  | processed   | error       | (!)
|----------+------+------+-------------+-------------|
| foo      | foo  | bar  | processed   | processed   |
| foo      | bar  | foo  | processed   | processed   |
| foo      | bar  | bar  | skipped     | skipped     |
| foo/bar  | foo  | bar  | processed   | processed   |
| foo/bar  | foo  | buz  | skipped     | skipped     |
| foo/bar  | bar  | bar  | skipped     | skipped     |
| foo/     | foo  | bar  | processed   | processed   |
| foo/     | bar  | bar  | skipped     | skipped     |
| /bar     | foo  | bar  | processed   | processed   |
| /bar     | foo  | foo  | skipped     | skipped     |
| /        | foo  | bar  | processed   | error       | (!)
|          | foo  | bar  | skipped     | error       | (!)

Fixes: 10b1b3f3e56a ("selftests/bpf: consolidate and improve file/prog filtering in veristat")
Signed-off-by: Eduard Zingerman <[email protected]>
---
 tools/testing/selftests/bpf/veristat.c | 76 ++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index c9c257784ee3..6aaa06790daa 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -514,6 +514,40 @@ static bool is_bpf_obj_file(const char *path) {
 	return err == 0;
 }
 
+/* Exact filter match */
+static bool name_filter_matches(struct filter *f, const char *filename, const char *prog_name)
+{
+	if (f->any_glob)
+		return glob_matches(filename, f->any_glob) ||
+		       (prog_name && glob_matches(prog_name, f->any_glob));
+	if (f->file_glob && f->prog_glob)
+		return prog_name &&
+		       glob_matches(filename, f->file_glob) &&
+		       glob_matches(prog_name, f->prog_glob);
+	if (f->file_glob)
+		return glob_matches(filename, f->file_glob);
+	if (f->prog_glob)
+		return prog_name && glob_matches(prog_name, f->prog_glob);
+	return false;
+}
+
+/* Check if the filter does not outright reject the file name */
+static bool name_filter_may_match(struct filter *f, const char *filename)
+{
+	if (f->file_glob)
+		return glob_matches(filename, f->file_glob);
+	/*
+	 * If we don't know program name yet, any_glob filter
+	 * has to assume that current BPF object file might be
+	 * relevant; we'll check again later on after opening
+	 * BPF object file, at which point program name will
+	 * be known finally.
+	 */
+	if (f->any_glob || f->prog_glob)
+		return true;
+	return false;
+}
+
 static bool should_process_file_prog(const char *filename, const char *prog_name)
 {
 	struct filter *f;
@@ -521,16 +555,7 @@ static bool should_process_file_prog(const char *filename, const char *prog_name
 
 	for (i = 0; i < env.deny_filter_cnt; i++) {
 		f = &env.deny_filters[i];
-		if (f->kind != FILTER_NAME)
-			continue;
-
-		if (f->any_glob && glob_matches(filename, f->any_glob))
-			return false;
-		if (f->any_glob && prog_name && glob_matches(prog_name, f->any_glob))
-			return false;
-		if (f->file_glob && glob_matches(filename, f->file_glob))
-			return false;
-		if (f->prog_glob && prog_name && glob_matches(prog_name, f->prog_glob))
+		if (f->kind == FILTER_NAME && name_filter_matches(f, filename, prog_name))
 			return false;
 	}
 
@@ -540,24 +565,15 @@ static bool should_process_file_prog(const char *filename, const char *prog_name
 			continue;
 
 		allow_cnt++;
-		if (f->any_glob) {
-			if (glob_matches(filename, f->any_glob))
-				return true;
-			/* If we don't know program name yet, any_glob filter
-			 * has to assume that current BPF object file might be
-			 * relevant; we'll check again later on after opening
-			 * BPF object file, at which point program name will
-			 * be known finally.
-			 */
-			if (!prog_name || glob_matches(prog_name, f->any_glob))
-				return true;
-		} else {
-			if (f->file_glob && !glob_matches(filename, f->file_glob))
-				continue;
-			if (f->prog_glob && prog_name && !glob_matches(prog_name, f->prog_glob))
-				continue;
+		if (prog_name && name_filter_matches(f, filename, prog_name))
+			return true;
+		/*
+		 * If there is no prog_name and the file name is not blocked by
+		 * the filter, allow to open the file. Afterwards there would be
+		 * a second refining query with prog_name set.
+		 */
+		if (!prog_name && name_filter_may_match(f, filename))
 			return true;
-		}
 	}
 
 	/* if there are no file/prog name allow filters, allow all progs,
@@ -703,6 +719,12 @@ static int append_filter(struct filter **filters, int *cnt, const char *str)
 		}
 	}
 
+	if ((!f->any_glob && !f->file_glob && !f->prog_glob) ||
+	    (f->any_glob && strcmp(f->any_glob, "") == 0)) {
+		fprintf(stderr, "Invalid filter: '%s'\n", str);
+		return -EINVAL;
+	}
+
 	*cnt += 1;
 	return 0;
 }

-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.