Merge branch 'selftests-bpf-fix-for-veristat-file-prog-filters-processing'

Eduard Zingerman says:

====================
selftests/bpf: fix for veristat file/prog filters processing

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'. Fix the expression to exclude only a
program 'prog' from a file 'file', also add a set of tests to exercise
filtering logic.

Changelog:
v1 -> v2:
- added fixes tag for patch #1 (bot+bpf-ci);
- extended test cases for '!*foo*' and '*foo*' filters in patch #2
  (bot+bpf-ci);
- added patch #3, replacing direct read() calls with calls to
  read_output(), guaranteeing input buffer null termination
  (bot+bpf-ci).

v1: https://lore.kernel.org/bpf/20260811-veristat-filter-fix-v1-0-b5b43c431550@gmail.com/
---
====================

Link: https://patch.msgid.link/20260811-veristat-filter-fix-v2-0-6c234c4cd6ef@gmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

tools/testing/selftests/bpf/prog_tests/test_veristat.c

@@ -37,6 +37,14 @@ static struct fixture *init_fixture(void)
 	return fix;
 }
 
+static void read_output(struct fixture *fix)
+{
+	ssize_t len = pread(fix->fd, fix->output, fix->sz - 1, 0);
+
+	fix->output[len < 0 ? 0 : len] = 0;
+	ASSERT_GE(len, 0, "pread");
+}
+
 static void teardown_fixture(struct fixture *fix)
 {
 	free(fix->output);
@@ -74,7 +82,7 @@ static void test_set_global_vars_succeeds(void)
 	    " -G \"struct11 [ 7 ] [ 5 ] .struct2[0][1].u.mat[3][0] = 175\" " \
 	    " -vl2 > %s", fix->veristat, fix->tmpfile);
 
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("=0xf000000000000001 ", "var_s64 = 0xf000000000000001");
 	__CHECK_STR("=0xfedcba9876543210 ", "var_u64 = 0xfedcba9876543210");
 	__CHECK_STR("=0x80000000 ", "var_s32 = -0x80000000");
@@ -116,7 +124,7 @@ static void test_set_global_vars_from_file_succeeds(void)
 	syncfs(fd);
 	SYS(out, "%s set_global_vars.bpf.o -G \"@%s\" -vl2 > %s",
 	    fix->veristat, input_file, fix->tmpfile);
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("=0x8000 ", "var_s16 = -32768");
 	__CHECK_STR("=0xecec ", "var_u16 = 60652");
 
@@ -134,7 +142,7 @@ static void test_set_global_vars_out_of_range(void)
 		 "%s set_global_vars.bpf.o -G \"var_s32 = 2147483648\" -vl2 2> %s",
 		 fix->veristat, fix->tmpfile);
 
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("is out of range [-2147483648; 2147483647]", "out of range");
 
 out:
@@ -149,7 +157,7 @@ static void test_unsupported_ptr_array_type(void)
 		 "%s set_global_vars.bpf.o -G \"ptr_arr[0] = 0\" -vl2 2> %s",
 		 fix->veristat, fix->tmpfile);
 
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("Can't set ptr_arr[0]. Only ints and enums are supported", "ptr_arr");
 
 out:
@@ -164,7 +172,7 @@ static void test_array_out_of_bounds(void)
 		 "%s set_global_vars.bpf.o -G \"arr[99] = 0\" -vl2 2> %s",
 		 fix->veristat, fix->tmpfile);
 
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("Array index 99 is out of bounds", "arr[99]");
 
 out:
@@ -179,7 +187,7 @@ static void test_array_index_not_found(void)
 		 "%s set_global_vars.bpf.o -G \"arr[EG2] = 0\" -vl2 2> %s",
 		 fix->veristat, fix->tmpfile);
 
-	read(fix->fd, fix->output, fix->sz);
+	read_output(fix);
 	__CHECK_STR("Can't resolve enum value EG2", "arr[EG2]");
 
 out:
@@ -230,6 +238,97 @@ static void test_no_array_index_for_array(void)
 	teardown_fixture(fix);
 }
 
+/*
+ * Name filter tests below run veristat on veristat_foo.bpf.o and
+ * veristat_bar.bpf.o, both defining programs 'foo', 'bar' and 'buz'.
+ * Every entry describes a single (filters, file, prog) combination and
+ * tells whether that program is expected in the veristat output:
+ * 'true' if it is, 'false' if it is not and -1 if veristat is expected
+ * to reject the filter.
+ */
+#define FILTER_OBJS "veristat_foo.bpf.o veristat_bar.bpf.o"
+
+static const struct name_filter_case {
+	const char *filters;
+	const char *file;
+	const char *prog;
+	int included;
+} name_filter_cases[] = {
+	/* no filters, every program is processed */
+	{ "",			"foo", "foo", true  },
+	{ "",			"foo", "bar", true  },
+	{ "",			"foo", "buz", true  },
+	{ "",			"bar", "foo", true  },
+	{ "",			"bar", "bar", true  },
+	{ "",			"bar", "buz", true  },
+	/* deny filters */
+	{ "-f '!*foo*'",	"foo", "bar", false },
+	{ "-f '!*foo*'",	"bar", "foo", false },
+	{ "-f '!*foo*'",	"bar", "bar", true  },
+	{ "-f '!*foo*/bar'",	"foo", "bar", false },
+	{ "-f '!*foo*/bar'",	"foo", "buz", true  },
+	{ "-f '!*foo*/bar'",	"bar", "bar", true  },
+	{ "-f '!*foo*/'",	"foo", "bar", false },
+	{ "-f '!*foo*/'",	"bar", "bar", true  },
+	{ "-f '!/bar'",		"foo", "bar", false },
+	{ "-f '!/bar'",		"foo", "foo", true  },
+	{ "-f '!/'",		"foo", "bar", -1    },
+	{ "-f '!'",		"foo", "bar", -1    },
+	/* allow filters */
+	{ "-f '*foo*'",		"foo", "bar", true  },
+	{ "-f '*foo*'",		"bar", "foo", true  },
+	{ "-f '*foo*'",		"bar", "bar", false },
+	{ "-f '*foo*/bar'",	"foo", "bar", true  },
+	{ "-f '*foo*/bar'",	"foo", "buz", false },
+	{ "-f '*foo*/bar'",	"bar", "bar", false },
+	{ "-f '*foo*/'",	"foo", "bar", true  },
+	{ "-f '*foo*/'",	"bar", "bar", false },
+	{ "-f '/bar'",		"foo", "bar", true  },
+	{ "-f '/bar'",		"foo", "foo", false },
+	{ "-f '/'",		"foo", "bar", -1    },
+	{ "-f ''",		"foo", "bar", -1    },
+	/* allow and deny filters combined */
+	{ "-f '*foo*/' -f '!/bar'", "foo", "foo", true  },
+	{ "-f '*foo*/' -f '!/bar'", "foo", "bar", false },
+	{ "-f '*foo*/' -f '!/bar'", "bar", "foo", false },
+};
+
+static void test_name_filters(void)
+{
+	struct fixture *fix = init_fixture();
+	const struct name_filter_case *t;
+	char cmd[512], row[64], name[128];
+	int i, err;
+
+	for (i = 0; i < ARRAY_SIZE(name_filter_cases); i++) {
+		t = &name_filter_cases[i];
+		/* stderr is merged with stdout in order to catch error messages */
+		snprintf(cmd, sizeof(cmd), "%s " FILTER_OBJS " -q -o csv -e file,prog %s > %s 2>&1",
+			 fix->veristat, t->filters, fix->tmpfile);
+		err = system(cmd);
+		read_output(fix);
+
+		snprintf(row, sizeof(row), "veristat_%s.bpf.o,%s", t->file, t->prog);
+		snprintf(name, sizeof(name), "veristat %s: %s", t->filters, row);
+		switch (t->included) {
+		case true:
+			ASSERT_OK(err, name);
+			ASSERT_HAS_SUBSTR(fix->output, row, name);
+			break;
+		case false:
+			ASSERT_OK(err, name);
+			ASSERT_FALSE(!!strstr(fix->output, row), name);
+			break;
+		case -1:
+			ASSERT_NEQ(err, 0, name);
+			ASSERT_HAS_SUBSTR(fix->output, "Invalid filter", name);
+			break;
+		}
+	}
+
+	teardown_fixture(fix);
+}
+
 void test_veristat(void)
 {
 	if (test__start_subtest("set_global_vars_succeeds"))
@@ -256,6 +355,8 @@ void test_veristat(void)
 	if (test__start_subtest("test_no_array_index_for_array"))
 		test_no_array_index_for_array();
 
+	if (test__start_subtest("name_filters"))
+		test_name_filters();
 }
 
 #undef __CHECK_STR

tools/testing/selftests/bpf/progs/veristat_bar.c

@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include "veristat_foo.c"

tools/testing/selftests/bpf/progs/veristat_foo.c

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+/*
+ * Programs below exist only to exercise veristat's -f name filters,
+ * their bodies are irrelevant, only the names matter.
+ * This file is also included by veristat_bar.c, so that the same set of
+ * program names is available in two differently named object files.
+ */
+
+SEC("socket")
+int foo(void *ctx)
+{
+	return 0;
+}
+
+SEC("socket")
+int bar(void *ctx)
+{
+	return 0;
+}
+
+SEC("socket")
+int buz(void *ctx)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";

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;
 }