Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] bpf: stack OOB access via unterminated strncpy() - [Low] bpf: off-by-one snprintf check and file leak --
commit 87f9013a78d7a34904987bd2e9122ad081e635b2 Author: Gabriele Monaco <[email protected]> tools/rv: Implement BPF monitor discovery and listing Implement the BPF monitor listing functionality that scans for compiled BPF monitor object files and displays them in the monitor list. > diff --git a/tools/verification/rv/src/bpf_monitor.c > b/tools/verification/rv/src/bpf_monitor.c > new file mode 100644 > index 0000000000000..a1de0c1577128 > --- /dev/null > +++ b/tools/verification/rv/src/bpf_monitor.c [ ... ] > +static void bpf_object_iterate_path(const char *base_path, const char > *subdir, > + void (*action)(const char *name, struct > bpf_object *obj)) > +{ > + char path[MAX_PATH]; > + struct dirent *entry; > + DIR *dir; > + char *ext; > + > + snprintf(path, sizeof(path), "%s/%s", base_path, subdir); > + dir = opendir(path); > + if (!dir) { > + debug_msg("bpf: error opening directory: %s\n", path); > + return; > + } > + > + while ((entry = readdir(dir)) != NULL) { > + size_t size; > + struct bpf_object *obj; > + char name[MAX_DA_NAME_LEN], obj_path[MAX_PATH]; > + > + if (entry->d_name[0] == '.') > + continue; > + > + ext = strrchr(entry->d_name, '.'); > + if (!ext || strcmp(ext, ".o") != 0) > + continue; > + > + size = snprintf(obj_path, sizeof(obj_path), "%s/%s", path, > + entry->d_name); > + obj = bpf_object__open_file(obj_path, NULL); > + if (!obj || size > MAX_PATH) { [Severity: Low] Does this logic correctly handle path truncation? The check size > MAX_PATH has an off-by-one error since snprintf() returns the number of characters that would have been written excluding the null byte, meaning truncation occurs if size >= MAX_PATH (or sizeof(obj_path)). Also, bpf_object__open_file() is called before this check, potentially operating on truncated paths. If open_file succeeds on a truncated string, would the obj resource be leaked on the error path via the continue statement? > + err_msg("bpf: error opening object file %s: %s\n", > + obj_path, strerror(errno)); > + continue; > + } > + > + strncpy(name, entry->d_name, sizeof(name)); > + ext = strrchr(name, '.'); > + if (ext) > + *ext = '\0'; [Severity: Medium] Could this sequence result in a stack out-of-bounds read and write? If entry->d_name is exactly MAX_DA_NAME_LEN characters or longer, strncpy() will leave the name array without a null terminator. This causes strrchr() to scan adjacent stack memory (such as obj_path) to find a '.', and potentially write a '\0' out of bounds if one is found. Can this be triggered by a local user placing a file with a long name in the bpf_monitors directory? > + > + action(name, obj); > + > + bpf_object__close(obj); > + } > + > + closedir(dir); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
