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